Skip to content

Commit 9e9eb74

Browse files
MeirShpilraienMeirShpilraien
authored
add multi result support of json.get (RedisJSON#408)
* add multi result support of json.get * fmt fixes * extend the test
1 parent e146eda commit 9e9eb74

File tree

4 files changed

+56
-32
lines changed

4 files changed

+56
-32
lines changed

‎src/lib.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -227,14 +227,12 @@ fn json_get(ctx: &Context, args: Vec<String>) -> RedisResult {
227227

228228
// path is optional -> no path found we use root "$"
229229
if paths.is_empty() {
230-
paths.push(Path::new(JSON_ROOT_PATH.to_string()));
230+
paths.push(Path::new(".".to_string()));
231231
}
232232

233233
let key = ctx.open_key_writable(&key);
234234
let value = match key.get_value::<RedisJSON>(&REDIS_JSON_TYPE)? {
235-
Some(doc) => doc
236-
.to_json(&mut paths, indent, newline, space, format)?
237-
.into(),
235+
Some(doc) => doc.to_json(&mut paths, indent, newline, space, format)?,
238236
None => RedisValue::Null,
239237
};
240238

‎src/redisjson.rs

+38-21
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use crate::REDIS_JSON_TYPE_VERSION;
1414
use bson::decode_document;
1515
use jsonpath_lib::SelectorMut;
1616
use redis_module::raw::{self, Status};
17+
use redis_module::RedisValue;
1718
use serde::Serialize;
1819
use serde_json::{Map, Value};
1920
use std::io::Cursor;
@@ -48,12 +49,15 @@ impl Format {
4849
pub struct Path {
4950
pub path: String,
5051
pub fixed: String,
52+
pub is_legacy: bool,
5153
}
5254

5355
impl Path {
5456
pub fn new(path: String) -> Path {
5557
let mut fixed = path.clone();
58+
let mut is_legacy = false;
5659
if !fixed.starts_with('$') {
60+
is_legacy = true;
5761
if fixed == "." {
5862
fixed.replace_range(..1, "$");
5963
} else if fixed.starts_with('.') {
@@ -62,7 +66,11 @@ impl Path {
6266
fixed.insert_str(0, "$.");
6367
}
6468
}
65-
Path { path, fixed }
69+
Path {
70+
path,
71+
fixed,
72+
is_legacy,
73+
}
6674
}
6775
}
6876

@@ -240,22 +248,33 @@ impl RedisJSON {
240248
Ok(res)
241249
}
242250

251+
fn serialize_json(&self, val: &Value, indent: &str, newline: &str, space: &str) -> String {
252+
let formatter =
253+
RedisJsonFormatter::new(indent.as_bytes(), space.as_bytes(), newline.as_bytes());
254+
255+
let mut out = serde_json::Serializer::with_formatter(Vec::new(), formatter);
256+
val.serialize(&mut out).unwrap();
257+
String::from_utf8(out.into_inner()).unwrap()
258+
}
259+
243260
pub fn to_json(
244261
&self,
245262
paths: &mut Vec<Path>,
246263
indent: String,
247264
newline: String,
248265
space: String,
249266
format: Format,
250-
) -> Result<String, Error> {
251-
let temp_doc;
252-
let res = if paths.len() > 1 {
267+
) -> Result<RedisValue, Error> {
268+
if format == Format::BSON {
269+
return Err("Soon to come...".into());
270+
}
271+
if paths.len() > 1 {
253272
let mut selector = jsonpath_lib::selector(&self.data);
254273
// TODO: Creating a temp doc here duplicates memory usage. This can be very memory inefficient.
255274
// A better way would be to create a doc of references to the original doc but no current support
256275
// in serde_json. I'm going for this implementation anyway because serde_json isn't supposed to be
257276
// memory efficient and we're using it anyway. See https://github.com/serde-rs/json/issues/635.
258-
temp_doc = Value::Object(paths.drain(..).fold(Map::new(), |mut acc, path| {
277+
let temp_doc = Value::Object(paths.drain(..).fold(Map::new(), |mut acc, path| {
259278
let value = match selector(&path.fixed) {
260279
Ok(s) => match s.first() {
261280
Some(v) => v,
@@ -266,24 +285,22 @@ impl RedisJSON {
266285
acc.insert(path.path, (*value).clone());
267286
acc
268287
}));
269-
&temp_doc
288+
Ok(self
289+
.serialize_json(&temp_doc, &indent, &newline, &space)
290+
.into())
270291
} else {
271-
self.get_first(&paths[0].fixed)?
272-
};
273-
274-
match format {
275-
Format::JSON => {
276-
let formatter = RedisJsonFormatter::new(
277-
indent.as_bytes(),
278-
space.as_bytes(),
279-
newline.as_bytes(),
280-
);
281-
282-
let mut out = serde_json::Serializer::with_formatter(Vec::new(), formatter);
283-
res.serialize(&mut out).unwrap();
284-
Ok(String::from_utf8(out.into_inner()).unwrap())
292+
let path = &paths[0];
293+
if path.is_legacy {
294+
let res = self.get_first(&path.fixed)?;
295+
Ok(self.serialize_json(res, &indent, &newline, &space).into())
296+
} else {
297+
Ok(self
298+
.get_values(&path.fixed)?
299+
.drain(..)
300+
.map(|v| self.serialize_json(v, &indent, &newline, &space))
301+
.collect::<Vec<String>>()
302+
.into())
285303
}
286-
Format::BSON => Err("Soon to come...".into()), //results.into() as Bson,
287304
}
288305
}
289306

‎tests/pytest/test.py

+15-6
Original file line numberDiff line numberDiff line change
@@ -439,10 +439,10 @@ def testClear(env):
439439

440440
# Make sure specific obj content exists before clear
441441
obj_content = r'{"a":1,"b":2}'
442-
r.expect('JSON.GET', 'test', '$.arr[2].n').equal(obj_content)
442+
r.expect('JSON.GET', 'test', '$.arr[2].n').equal([obj_content])
443443
# Make sure specific arr content exists before clear
444444
arr_content = r'["to","be","cleared",4]'
445-
r.expect('JSON.GET', 'test', '$.arr[3].n2.n').equal(arr_content)
445+
r.expect('JSON.GET', 'test', '$.arr[3].n2.n').equal([arr_content])
446446

447447
# Clear obj and arr with specific paths
448448
r.expect('JSON.CLEAR', 'test', '$.arr[2].n').equal(1)
@@ -452,9 +452,9 @@ def testClear(env):
452452
r.expect('JSON.CLEAR', 'test', '$.arr[1]').equal(0)
453453

454454
# Make sure specific obj content was cleared
455-
r.expect('JSON.GET', 'test', '$.arr[2].n').equal('{}')
455+
r.expect('JSON.GET', 'test', '$.arr[2].n').equal(['{}'])
456456
# Make sure specific arr content was cleared
457-
r.expect('JSON.GET', 'test', '$.arr[3].n2.n').equal('[]')
457+
r.expect('JSON.GET', 'test', '$.arr[3].n2.n').equal(['[]'])
458458

459459
# Make sure only appropriate content (obj and arr) was cleared - and that errors were printed for inappropriate content (string and numeric)
460460
# TODO: Enable when supporting multi results (and not only the first)
@@ -463,11 +463,11 @@ def testClear(env):
463463
# Clear root
464464
# TODO: switch order of the following paths and expect .equals(2) when supporting multi-paths
465465
r.expect('JSON.CLEAR', 'test', '$', '$.arr[2].n').equal(1)
466-
r.expect('JSON.GET', 'test', '$').equal('{}')
466+
r.expect('JSON.GET', 'test', '$').equal(['{}'])
467467

468468
r.expect('JSON.SET', 'test', '$', obj_content).ok()
469469
r.expect('JSON.CLEAR', 'test').equal(1)
470-
r.expect('JSON.GET', 'test', '$').equal('{}')
470+
r.expect('JSON.GET', 'test', '$').equal(['{}'])
471471

472472
def testArrayCRUD(env):
473473
"""Test JSON Array CRUDness"""
@@ -829,6 +829,15 @@ def testIssue_80(env):
829829
r.execute_command('JSON.GET', 'test', '$.[?(@.code=="2")]')
830830

831831

832+
def testMultiPathResults(env):
833+
env.expect("JSON.SET", "k", '$', '[1,2,3]').ok()
834+
env.expect("JSON.GET", "k", '$[*]').equal(['1', '2', '3'])
835+
env.expect("JSON.SET", "k", '$', '{"a":[1,2,3],"b":["c", "d", "e"],"c":"k"}').ok()
836+
env.expect("JSON.GET", "k", '$.*[0,2]').equal(['1', '3', '"c"', '"e"'])
837+
838+
# make sure legacy json path returns single result
839+
env.expect("JSON.GET", "k", '.*[0,2]').equal('1')
840+
832841
# class CacheTestCase(BaseReJSONTest):
833842
# @property
834843
# def module_args(env):

‎tests/pytest/test_keyspace_notifications.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ def test_keyspace_set(env):
3737
env.assertEqual(8, r.execute_command('JSON.STRLEN', 'test_key', '$.foo'))
3838
env.assertEqual(None, pubsub.get_message())
3939

40-
env.assertEqual('"gogototo"', r.execute_command('JSON.GET', 'test_key', '$.foo'))
40+
env.assertEqual(['"gogototo"'], r.execute_command('JSON.GET', 'test_key', '$.foo'))
4141
env.assertEqual(None, pubsub.get_message())
4242

4343
env.assertEqual(['"gogototo"', None], r.execute_command('JSON.MGET', 'test_key', 'test_key1', '$.foo'))

0 commit comments

Comments
 (0)