From 0e65ee80b6a0a895bbecfb3e8adc7f69bddd08c5 Mon Sep 17 00:00:00 2001 From: oshadmi Date: Wed, 17 Aug 2022 16:14:59 +0300 Subject: [PATCH 1/6] jsonpath_rs code review --- README.md | 2 +- src/grammer.pest | 2 +- src/json_node.rs | 15 +++++----- src/json_path.rs | 71 +++++++++++++++++++++----------------------- src/lib.rs | 2 +- tests/filter.rs | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ tests/paths.rs | 17 +++++++++++ 7 files changed, 139 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 00f7f30..4f6fed6 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ [![Forum](https://img.shields.io/badge/Forum-RedisJSON-blue)](https://forum.redislabs.com/c/modules/redisjson) [![Discord](https://img.shields.io/discord/697882427875393627?style=flat-square)](https://discord.gg/QUkjSsk) -A JSONPath library for rust. The idea behind this library is that it can operate on any json representation as long as it implements the [`SelectValue`](src/select_value.rs) triat. The library has an implementation for [serde_json value](https://docs.serde.rs/serde_json/value/enum.Value.html) and [ivalue](https://docs.rs/tch/0.1.1/tch/enum.IValue.html). +A JSONPath library for rust. The idea behind this library is that it can operate on any json representation as long as it implements the [`SelectValue`](src/select_value.rs) trait. The library has an implementation for [serde_json value](https://docs.serde.rs/serde_json/value/enum.Value.html) and [ivalue](https://docs.rs/tch/0.1.1/tch/enum.IValue.html). ### Getting Started Add the following to your cargo.toml diff --git a/src/grammer.pest b/src/grammer.pest index 515102d..90e1be1 100644 --- a/src/grammer.pest +++ b/src/grammer.pest @@ -1,4 +1,4 @@ -literal = @{('a' .. 'z' | 'A' .. 'Z' | '0' .. '9' | "-" | "_" | "`" | "~" | "+" | "#" | "%" | "$" | "^" | "/")+} +literal = @{('a' .. 'z' | 'A' .. 'Z' | '0' .. '9' | "-" | "_" | "`" | "~" | "+" | "#" | "%" | "$" | "^" | "/" | ":")+} string = _{("'" ~ (string_value) ~ "'") | ("\"" ~ (string_value) ~ "\"") | ("\"" ~ (string_value_escape_1) ~ "\"") | ("'" ~ (string_value_escape_2) ~ "'")} string_escape = @{"\\"} diff --git a/src/json_node.rs b/src/json_node.rs index a5d0262..3fe81d2 100644 --- a/src/json_node.rs +++ b/src/json_node.rs @@ -107,8 +107,8 @@ impl SelectValue for Value { fn get_long(&self) -> i64 { match self { Value::Number(n) => { - if n.is_i64() || n.is_u64() { - n.as_i64().unwrap() + if let Some(n) = n.as_i64() { + n } else { panic!("not a long"); } @@ -155,9 +155,10 @@ impl SelectValue for IValue { } fn contains_key(&self, key: &str) -> bool { - match self.as_object() { - Some(o) => o.contains_key(key), - _ => false, + if let Some(o) = self.as_object() { + o.contains_key(key) + } else { + false } } @@ -247,7 +248,7 @@ impl SelectValue for IValue { } } _ => { - panic!("not a long"); + panic!("not a number"); } } } @@ -262,7 +263,7 @@ impl SelectValue for IValue { } } _ => { - panic!("not a double"); + panic!("not a number"); } } } diff --git a/src/json_path.rs b/src/json_path.rs index cde10a1..aa60d29 100644 --- a/src/json_path.rs +++ b/src/json_path.rs @@ -30,6 +30,7 @@ pub struct QueryCompilationError { } impl<'i> Query<'i> { + #[allow(dead_code)] pub fn pop_last(&mut self) -> Option<(String, JsonPathToken)> { let last = self.root.next_back(); match last { @@ -56,17 +57,19 @@ impl<'i> Query<'i> { } } + #[allow(dead_code)] pub fn size(&mut self) -> usize { if self.size.is_some() { - return *self.size.as_ref().unwrap(); + return self.size.unwrap(); } self.is_static(); self.size() } + #[allow(dead_code)] pub fn is_static(&mut self) -> bool { if self.is_static.is_some() { - return *self.is_static.as_ref().unwrap(); + return self.is_static.unwrap(); } let mut size = 0; let mut is_static = true; @@ -264,33 +267,33 @@ enum PathTrackerElement<'i> { #[derive(Clone)] struct PathTracker<'i, 'j> { - father: Option<&'j PathTracker<'i, 'j>>, + parent: Option<&'j PathTracker<'i, 'j>>, element: PathTrackerElement<'i>, } -const fn create_empty_trucker<'i, 'j>() -> PathTracker<'i, 'j> { +const fn create_empty_tracker<'i, 'j>() -> PathTracker<'i, 'j> { PathTracker { - father: None, + parent: None, element: PathTrackerElement::Root, } } -const fn create_str_trucker<'i, 'j>( +const fn create_str_tracker<'i, 'j>( s: &'i str, - father: &'j PathTracker<'i, 'j>, + parent: &'j PathTracker<'i, 'j>, ) -> PathTracker<'i, 'j> { PathTracker { - father: Some(father), + parent: Some(parent), element: PathTrackerElement::Key(s), } } -const fn create_index_trucker<'i, 'j>( +const fn create_index_tracker<'i, 'j>( index: usize, - father: &'j PathTracker<'i, 'j>, + parent: &'j PathTracker<'i, 'j>, ) -> PathTracker<'i, 'j> { PathTracker { - father: Some(father), + parent: Some(parent), element: PathTrackerElement::Index(index), } } @@ -343,21 +346,23 @@ impl<'i, 'j, S: SelectValue> TermEvaluationResult<'i, 'j, S> { (TermEvaluationResult::String(s1), TermEvaluationResult::String(s2)) => { CmpResult::Ord(s1.cmp(s2)) } + (TermEvaluationResult::Bool(b1), TermEvaluationResult::Bool(b2)) => { + CmpResult::Ord(b1.cmp(b2)) + } (TermEvaluationResult::Value(v), _) => match v.get_type() { SelectValueType::Long => TermEvaluationResult::Integer(v.get_long()).cmp(s), SelectValueType::Double => TermEvaluationResult::Float(v.get_double()).cmp(s), SelectValueType::String => TermEvaluationResult::Str(v.as_str()).cmp(s), + SelectValueType::Bool => TermEvaluationResult::Bool(v.get_bool()).cmp(s), _ => CmpResult::NotCmparable, }, (_, TermEvaluationResult::Value(v)) => match v.get_type() { SelectValueType::Long => self.cmp(&TermEvaluationResult::Integer(v.get_long())), SelectValueType::Double => self.cmp(&TermEvaluationResult::Float(v.get_double())), SelectValueType::String => self.cmp(&TermEvaluationResult::Str(v.as_str())), + SelectValueType::Bool => self.cmp(&TermEvaluationResult::Bool(v.get_bool())), _ => CmpResult::NotCmparable, }, - (TermEvaluationResult::Invalid, _) | (_, TermEvaluationResult::Invalid) => { - CmpResult::NotCmparable - } (_, _) => CmpResult::NotCmparable, } } @@ -391,16 +396,6 @@ impl<'i, 'j, S: SelectValue> TermEvaluationResult<'i, 'j, S> { fn eq(&self, s: &Self) -> bool { match (self, s) { - (TermEvaluationResult::Bool(b1), TermEvaluationResult::Bool(b2)) => *b1 == *b2, - (TermEvaluationResult::Value(v1), TermEvaluationResult::Bool(b2)) => { - if v1.get_type() == SelectValueType::Bool { - let b1 = v1.get_bool(); - b1 == *b2 - } else { - false - } - } - (TermEvaluationResult::Bool(_), TermEvaluationResult::Value(_)) => s.eq(self), (TermEvaluationResult::Value(v1), TermEvaluationResult::Value(v2)) => v1 == v2, (_, _) => match self.cmp(s) { CmpResult::Ord(o) => o.is_eq(), @@ -444,6 +439,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { } } + #[allow(dead_code)] pub fn create_with_generator( query: &'i Query<'i>, tracker_generator: UPTG, @@ -469,14 +465,14 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { self.calc_internal( pairs.clone(), val, - Some(create_str_trucker(key, &pt)), + Some(create_str_tracker(key, &pt)), calc_data, false, ); self.calc_full_scan( pairs.clone(), val, - Some(create_str_trucker(key, &pt)), + Some(create_str_tracker(key, &pt)), calc_data, ); } @@ -495,14 +491,14 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { self.calc_internal( pairs.clone(), v, - Some(create_index_trucker(i, &pt)), + Some(create_index_tracker(i, &pt)), calc_data, false, ); self.calc_full_scan( pairs.clone(), v, - Some(create_index_trucker(i, &pt)), + Some(create_index_tracker(i, &pt)), calc_data, ); } @@ -529,7 +525,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { if let Some(pt) = path_tracker { let items = json.items().unwrap(); for (key, val) in items { - let new_tracker = Some(create_str_trucker(key, &pt)); + let new_tracker = Some(create_str_tracker(key, &pt)); self.calc_internal(pairs.clone(), val, new_tracker, calc_data, true); } } else { @@ -543,7 +539,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { let values = json.values().unwrap(); if let Some(pt) = path_tracker { for (i, v) in values.enumerate() { - let new_tracker = Some(create_index_trucker(i, &pt)); + let new_tracker = Some(create_index_tracker(i, &pt)); self.calc_internal(pairs.clone(), v, new_tracker, calc_data, true); } } else { @@ -567,7 +563,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { let curr_val = json.get_key(curr.as_str()); if let Some(e) = curr_val { if let Some(pt) = path_tracker { - let new_tracker = Some(create_str_trucker(curr.as_str(), &pt)); + let new_tracker = Some(create_str_tracker(curr.as_str(), &pt)); self.calc_internal(pairs, e, new_tracker, calc_data, true); } else { self.calc_internal(pairs, e, None, calc_data, true); @@ -597,7 +593,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { _ => panic!("{}", format!("{:?}", c)), }; if let Some(e) = curr_val { - let new_tracker = Some(create_str_trucker(s, &pt)); + let new_tracker = Some(create_str_tracker(s, &pt)); self.calc_internal(pairs.clone(), e, new_tracker, calc_data, true); } } @@ -646,7 +642,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { let i = self.calc_abs_index(c.as_str().parse::().unwrap(), n); let curr_val = json.get_index(i); if let Some(e) = curr_val { - let new_tracker = Some(create_index_trucker(i, &pt)); + let new_tracker = Some(create_index_tracker(i, &pt)); self.calc_internal(pairs.clone(), e, new_tracker, calc_data, true); } } @@ -724,7 +720,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { for i in (start..end).step_by(step) { let curr_val = json.get_index(i); if let Some(e) = curr_val { - let new_tracker = Some(create_index_trucker(i, &pt)); + let new_tracker = Some(create_index_tracker(i, &pt)); self.calc_internal(pairs.clone(), e, new_tracker, calc_data, true); } } @@ -863,7 +859,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { } fn populate_path_tracker<'k, 'l>(&self, pt: &PathTracker<'l, 'k>, upt: &mut UPTG::PT) { - if let Some(f) = pt.father { + if let Some(f) = pt.parent { self.populate_path_tracker(f, upt); } match pt.element { @@ -920,7 +916,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { if let Some(pt) = path_tracker { for (i, v) in values.enumerate() { if self.evaluate_filter(curr.clone(), v, calc_data) { - let new_tracker = Some(create_index_trucker(i, &pt)); + let new_tracker = Some(create_index_tracker(i, &pt)); self.calc_internal( pairs.clone(), v, @@ -972,7 +968,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { self.calc_internal( root, json, - Some(create_empty_trucker()), + Some(create_empty_tracker()), &mut calc_data, true, ); @@ -996,6 +992,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { .collect() } + #[allow(dead_code)] pub fn calc_paths<'j: 'i, S: SelectValue>(&self, json: &'j S) -> Vec> { self.calc_with_paths(json) .into_iter() diff --git a/src/lib.rs b/src/lib.rs index 66ec7af..d368c3e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -48,7 +48,7 @@ pub fn create<'i>(query: &'i Query<'i>) -> PathCalculator<'i, DummyTrackerGenera /// Create a PathCalculator object. The path calculator can be re-used /// to calculate json paths on different jsons. /// Unlike create(), this function will return results with full path as PTracker object. -/// It is possible to create your own path tracker by implement the PTrackerGenerator triat. +/// It is possible to create your own path tracker by implement the PTrackerGenerator trait. pub fn create_with_generator<'i>(query: &'i Query<'i>) -> PathCalculator<'i, PTrackerGenerator> { PathCalculator::create_with_generator(query, PTrackerGenerator) } diff --git a/tests/filter.rs b/tests/filter.rs index 7758d67..5f460c4 100644 --- a/tests/filter.rs +++ b/tests/filter.rs @@ -319,3 +319,80 @@ fn unimplemented_in_filter() { json!([]), ); } + +// #[test] +// fn filter_nested() { +// setup(); + +// select_and_then_compare( +// "$.store.book[?(@.authors[?(@.lastName == 'Rees')])].title", +// json!([ +// { +// "store": { +// "book": [ +// { +// "authors": [ +// { +// "firstName": "Nigel", +// "lastName": "Rees" +// }, +// { +// "firstName": "Evelyn", +// "lastName": "Waugh" +// } +// ], +// "title": "Sayings of the Century" +// }, +// { +// "authors": [ +// { +// "firstName": "Herman", +// "lastName": "Melville" +// }, +// { +// "firstName": "Somebody", +// "lastName": "Else" +// } +// ], +// "title": "Moby Dick" +// } +// ] +// } +// } +// ]), +// json!(["Sayings of the Century"]), +// ); +// } + +// #[test] +// fn filter_inner() { +// setup(); + +// select_and_then_compare( +// "$['a'][?(@.inner.for.inner=='u8')].id", +// json!([ +// { +// "a": { +// "id": "0:4", +// "inner": { +// "for": {"inner": "u8", "kind": "primitive"} +// } +// } +// } +// ]), +// json!(["0:4"]), +// ); +// } + +#[test] +fn op_object_or_nonexisting_default() { + setup(); + + select_and_then_compare( + "$.friends[?(@.id >= 2 || @.id == 4)]", + read_json("./json_examples/data_obj.json"), + json!([ + { "id" : 2, "name" : "Gray Berry" } + ]), + ); +} diff --git a/tests/paths.rs b/tests/paths.rs index fe2878a..d3029c9 100644 --- a/tests/paths.rs +++ b/tests/paths.rs @@ -113,3 +113,20 @@ fn dolla_token_in_path() { ]), ); } + +#[test] +fn colon_token_in_path() { + setup(); + + let payload = json!({ + "prod:id": "G637", + "prod_name": "coffee table", + "price": 194 + }); + + select_and_then_compare("$.price", payload.clone(), json!([194])); + + select_and_then_compare("$.prod_name", payload.clone(), json!(["coffee table"])); + + select_and_then_compare("$.prod:id", payload.clone(), json!(["G637"])); +} From f3a37d2b93d11470cb648d95580ea362f607773d Mon Sep 17 00:00:00 2001 From: meir Date: Thu, 8 Sep 2022 16:50:40 +0300 Subject: [PATCH 2/6] Remove the need for flat_arrays_on_filter, always flat on arrays and objects. --- src/json_path.rs | 45 +++++++-------- src/lib.rs | 14 ++--- tests/filter.rs | 133 +++++++++++++++++++++---------------------- tests/op.rs | 38 ++++++------- tests/return_type.rs | 14 ++--- 5 files changed, 114 insertions(+), 130 deletions(-) diff --git a/src/json_path.rs b/src/json_path.rs index aa60d29..aed3a9c 100644 --- a/src/json_path.rs +++ b/src/json_path.rs @@ -467,7 +467,6 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { val, Some(create_str_tracker(key, &pt)), calc_data, - false, ); self.calc_full_scan( pairs.clone(), @@ -479,7 +478,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { } else { let values = json.values().unwrap(); for v in values { - self.calc_internal(pairs.clone(), v, None, calc_data, false); + self.calc_internal(pairs.clone(), v, None, calc_data); self.calc_full_scan(pairs.clone(), v, None, calc_data); } } @@ -493,7 +492,6 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { v, Some(create_index_tracker(i, &pt)), calc_data, - false, ); self.calc_full_scan( pairs.clone(), @@ -504,7 +502,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { } } else { for v in values { - self.calc_internal(pairs.clone(), v, None, calc_data, false); + self.calc_internal(pairs.clone(), v, None, calc_data); self.calc_full_scan(pairs.clone(), v, None, calc_data); } } @@ -526,12 +524,12 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { let items = json.items().unwrap(); for (key, val) in items { let new_tracker = Some(create_str_tracker(key, &pt)); - self.calc_internal(pairs.clone(), val, new_tracker, calc_data, true); + self.calc_internal(pairs.clone(), val, new_tracker, calc_data); } } else { let values = json.values().unwrap(); for v in values { - self.calc_internal(pairs.clone(), v, None, calc_data, true); + self.calc_internal(pairs.clone(), v, None, calc_data); } } } @@ -540,11 +538,11 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { if let Some(pt) = path_tracker { for (i, v) in values.enumerate() { let new_tracker = Some(create_index_tracker(i, &pt)); - self.calc_internal(pairs.clone(), v, new_tracker, calc_data, true); + self.calc_internal(pairs.clone(), v, new_tracker, calc_data); } } else { for v in values { - self.calc_internal(pairs.clone(), v, None, calc_data, true); + self.calc_internal(pairs.clone(), v, None, calc_data); } } } @@ -564,9 +562,9 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { if let Some(e) = curr_val { if let Some(pt) = path_tracker { let new_tracker = Some(create_str_tracker(curr.as_str(), &pt)); - self.calc_internal(pairs, e, new_tracker, calc_data, true); + self.calc_internal(pairs, e, new_tracker, calc_data); } else { - self.calc_internal(pairs, e, None, calc_data, true); + self.calc_internal(pairs, e, None, calc_data); } } } @@ -594,7 +592,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { }; if let Some(e) = curr_val { let new_tracker = Some(create_str_tracker(s, &pt)); - self.calc_internal(pairs.clone(), e, new_tracker, calc_data, true); + self.calc_internal(pairs.clone(), e, new_tracker, calc_data); } } } else { @@ -611,7 +609,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { _ => panic!("{}", format!("{:?}", c)), }; if let Some(e) = curr_val { - self.calc_internal(pairs.clone(), e, None, calc_data, true); + self.calc_internal(pairs.clone(), e, None, calc_data); } } } @@ -643,7 +641,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { let curr_val = json.get_index(i); if let Some(e) = curr_val { let new_tracker = Some(create_index_tracker(i, &pt)); - self.calc_internal(pairs.clone(), e, new_tracker, calc_data, true); + self.calc_internal(pairs.clone(), e, new_tracker, calc_data); } } } else { @@ -651,7 +649,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { let i = self.calc_abs_index(c.as_str().parse::().unwrap(), n); let curr_val = json.get_index(i); if let Some(e) = curr_val { - self.calc_internal(pairs.clone(), e, None, calc_data, true); + self.calc_internal(pairs.clone(), e, None, calc_data); } } } @@ -721,14 +719,14 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { let curr_val = json.get_index(i); if let Some(e) = curr_val { let new_tracker = Some(create_index_tracker(i, &pt)); - self.calc_internal(pairs.clone(), e, new_tracker, calc_data, true); + self.calc_internal(pairs.clone(), e, new_tracker, calc_data); } } } else { for i in (start..end).step_by(step) { let curr_val = json.get_index(i); if let Some(e) = curr_val { - self.calc_internal(pairs.clone(), e, None, calc_data, true); + self.calc_internal(pairs.clone(), e, None, calc_data); } } } @@ -763,7 +761,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { results: Vec::new(), root: json, }; - self.calc_internal(term.into_inner(), json, None, &mut calc_data, false); + self.calc_internal(term.into_inner(), json, None, &mut calc_data); if calc_data.results.len() == 1 { TermEvaluationResult::Value(calc_data.results.pop().unwrap().res) } else { @@ -783,7 +781,6 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { calc_data.root, None, &mut new_calc_data, - false, ); if new_calc_data.results.len() == 1 { TermEvaluationResult::Value(new_calc_data.results.pop().unwrap().res) @@ -881,7 +878,6 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { json: &'j S, path_tracker: Option>, calc_data: &mut PathCalculatorData<'j, S, UPTG::PT>, - flat_arrays_on_filter: bool, ) { let curr = pairs.next(); match curr { @@ -893,7 +889,6 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { json, path_tracker.clone(), calc_data, - false, ); self.calc_full_scan(pairs, json, path_tracker, calc_data); } @@ -909,7 +904,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { self.calc_range(pairs, curr, json, path_tracker, calc_data); } Rule::filter => { - if flat_arrays_on_filter && json.get_type() == SelectValueType::Array { + if json.get_type() == SelectValueType::Array || json.get_type() == SelectValueType::Object { /* lets expend the array, this is how most json path engines work. * Pesonally, I think this if should not exists. */ let values = json.values().unwrap(); @@ -922,19 +917,18 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { v, new_tracker, calc_data, - true, ); } } } else { for v in values { if self.evaluate_filter(curr.clone(), v, calc_data) { - self.calc_internal(pairs.clone(), v, None, calc_data, true); + self.calc_internal(pairs.clone(), v, None, calc_data); } } } } else if self.evaluate_filter(curr.clone(), json, calc_data) { - self.calc_internal(pairs, json, path_tracker, calc_data, true); + self.calc_internal(pairs, json, path_tracker, calc_data); } } Rule::EOI => { @@ -970,10 +964,9 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { json, Some(create_empty_tracker()), &mut calc_data, - true, ); } else { - self.calc_internal(root, json, None, &mut calc_data, true); + self.calc_internal(root, json, None, &mut calc_data); } calc_data.results.drain(..).collect() } diff --git a/src/lib.rs b/src/lib.rs index d368c3e..977078c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -370,17 +370,17 @@ mod json_path_tests { #[test] fn test_filter_and() { - verify_json!(path:"$[?@.foo[0] == 1 && @foo[1] == 2].foo[0,1,2]", json:{"foo":[1,2,3], "bar":[4,5,6]}, results:[1,2,3]); + verify_json!(path:"$[?@.foo[0] == 1 && @foo[1] == 2].foo[0,1,2]", json:[{"foo":[1,2,3], "bar":[4,5,6]}], results:[1,2,3]); } #[test] fn test_filter_or() { - verify_json!(path:"$[?@.foo[0] == 2 || @.bar[0] == 4].*[0,1,2]", json:{"foo":[1,2,3], "bar":[4,5,6]}, results:[1,2,3,4,5,6]); + verify_json!(path:"$[?@.foo[0] == 2 || @.bar[0] == 4].*[0,1,2]", json:[{"foo":[1,2,3], "bar":[4,5,6]}], results:[1,2,3,4,5,6]); } #[test] fn test_complex_filter() { - verify_json!(path:"$[?(@.foo[0] == 1 && @.foo[2] == 3)||(@.bar[0]==4&&@.bar[2]==6)].*[0,1,2]", json:{"foo":[1,2,3], "bar":[4,5,6]}, results:[1,2,3,4,5,6]); + verify_json!(path:"$[?(@.foo[0] == 1 && @.foo[2] == 3)||(@.bar[0]==4&&@.bar[2]==6)].*[0,1,2]", json:[{"foo":[1,2,3], "bar":[4,5,6]}], results:[1,2,3,4,5,6]); } #[test] @@ -395,9 +395,9 @@ mod json_path_tests { #[test] fn test_filter_with_all() { - verify_json!(path:"$.*.[?(@.code==\"2\")].code", json:[{"code":"1"},{"code":"2"}], results:["2"]); - verify_json!(path:"$.*[?(@.code==\"2\")].code", json:[{"code":"1"},{"code":"2"}], results:["2"]); - verify_json!(path:"$*[?(@.code==\"2\")].code", json:[{"code":"1"},{"code":"2"}], results:["2"]); + verify_json!(path:"$.*.[?(@.code==\"2\")].code", json:[[{"code":"1"},{"code":"2"}]], results:["2"]); + verify_json!(path:"$.*[?(@.code==\"2\")].code", json:[[{"code":"1"},{"code":"2"}]], results:["2"]); + verify_json!(path:"$*[?(@.code==\"2\")].code", json:[[{"code":"1"},{"code":"2"}]], results:["2"]); } #[test] @@ -416,7 +416,7 @@ mod json_path_tests { #[test] fn test_complex_filter_with_literal() { verify_json!(path:"$.foo[?@.a == @.b].boo[:]", - json:{"foo":{"boo":[1,2,3],"a":1,"b":1}}, + json:{"foo":[{"boo":[1,2,3],"a":1,"b":1}]}, results:[1,2,3]); } diff --git a/tests/filter.rs b/tests/filter.rs index 5f460c4..f4c97c7 100644 --- a/tests/filter.rs +++ b/tests/filter.rs @@ -85,7 +85,7 @@ fn filter_parent_with_matched_child() { setup(); select_and_then_compare( - "$.a[?(@.b.c == 1)]", + "$[?(@.b.c == 1)]", json!({ "a": { "b": { @@ -108,7 +108,7 @@ fn filter_parent_exist_child() { setup(); select_and_then_compare( - "$.a[?(@.b.c)]", + "$[?(@.b.c)]", json!({ "a": { "b": { @@ -270,9 +270,9 @@ fn bugs38_array_notation_in_filter() { select_and_then_compare( "$..key[?(@['subKey'] == 'subKey2')]", json!([ - {"key": {"seq": 1, "subKey": "subKey1"}}, - {"key": {"seq": 2, "subKey": "subKey2"}}, - {"key": 42}, + {"key": [{"seq": 1, "subKey": "subKey1"}]}, + {"key": [{"seq": 2, "subKey": "subKey2"}]}, + {"key": [42]}, {"some": "value"} ]), json!([{"seq": 2, "subKey": "subKey2"}]), @@ -320,69 +320,66 @@ fn unimplemented_in_filter() { ); } -// #[test] -// fn filter_nested() { -// setup(); - -// select_and_then_compare( -// "$.store.book[?(@.authors[?(@.lastName == 'Rees')])].title", -// json!([ -// { -// "store": { -// "book": [ -// { -// "authors": [ -// { -// "firstName": "Nigel", -// "lastName": "Rees" -// }, -// { -// "firstName": "Evelyn", -// "lastName": "Waugh" -// } -// ], -// "title": "Sayings of the Century" -// }, -// { -// "authors": [ -// { -// "firstName": "Herman", -// "lastName": "Melville" -// }, -// { -// "firstName": "Somebody", -// "lastName": "Else" -// } -// ], -// "title": "Moby Dick" -// } -// ] -// } -// } -// ]), -// json!(["Sayings of the Century"]), -// ); -// } - -// #[test] -// fn filter_inner() { -// setup(); - -// select_and_then_compare( -// "$['a'][?(@.inner.for.inner=='u8')].id", -// json!([ -// { -// "a": { -// "id": "0:4", -// "inner": { -// "for": {"inner": "u8", "kind": "primitive"} -// } -// } -// } -// ]), -// json!(["0:4"]), -// ); -// } +#[test] +fn filter_nested() { + setup(); + + select_and_then_compare( + "$.store.book[?(@.authors[?(@.lastName == 'Rees')])].title", + json!({ + "store": { + "book": [ + { + "authors": [ + { + "firstName": "Nigel", + "lastName": "Rees" + }, + { + "firstName": "Evelyn", + "lastName": "Waugh" + } + ], + "title": "Sayings of the Century" + }, + { + "authors": [ + { + "firstName": "Herman", + "lastName": "Melville" + }, + { + "firstName": "Somebody", + "lastName": "Else" + } + ], + "title": "Moby Dick" + } + ] + } + }), + json!(["Sayings of the Century"]), + ); +} + +#[test] +fn filter_inner() { + setup(); + + select_and_then_compare( + "$[?(@.inner.for.inner=='u8')].id", + json!( + { + "a": { + "id": "0:4", + "inner": { + "for": {"inner": "u8", "kind": "primitive"} + } + } + }), + json!(["0:4"]), + ); +} #[test] fn op_object_or_nonexisting_default() { diff --git a/tests/op.rs b/tests/op.rs index 226bec4..17f430d 100644 --- a/tests/op.rs +++ b/tests/op.rs @@ -10,15 +10,13 @@ fn op_object_eq() { setup(); select_and_then_compare( - "$.school[?(@.friends == @.friends)]", + "$.school[?(@ == @)]", read_json("./json_examples/data_obj.json"), - json!([{ - "friends": [ + json!([[ {"id": 0, "name": "Millicent Norman"}, {"id": 1, "name": "Vincent Cannon" }, {"id": 2, "name": "Gray Berry"} - ] - }]), + ]]), ); } @@ -136,42 +134,42 @@ fn op_ge() { fn op_eq_for_number() { setup(); - select_and_then_compare("$.[?(@.a == 1)]", json!({ "a": 1 }), json!([{ "a": 1 }])); + select_and_then_compare("$.[?(@ == 1)]", json!({ "a": 1 }), json!([1])); } #[test] fn op_ne_for_number() { setup(); - select_and_then_compare("$.[?(@.a != 2)]", json!({ "a": 1 }), json!([{ "a": 1 }])); + select_and_then_compare("$.[?(@ != 2)]", json!({ "a": 1 }), json!([1])); } #[test] fn op_lt_for_number() { setup(); - select_and_then_compare("$.[?(@.a < 2)]", json!({ "a": 1 }), json!([{ "a": 1 }])); + select_and_then_compare("$.[?(@ < 2)]", json!({ "a": 1 }), json!([1])); } #[test] fn op_le_for_number() { setup(); - select_and_then_compare("$.[?(@.a <= 1)]", json!({ "a": 1 }), json!([{ "a": 1 }])); + select_and_then_compare("$.[?(@ <= 1)]", json!({ "a": 1 }), json!([1])); } #[test] fn op_gt_for_number() { setup(); - select_and_then_compare("$.[?(@.a > 0)]", json!({ "a": 1 }), json!([{ "a": 1 }])); + select_and_then_compare("$.[?(@ > 0)]", json!({ "a": 1 }), json!([1])); } #[test] fn op_ge_for_number() { setup(); - select_and_then_compare("$.[?(@.a >= 0)]", json!({ "a": 1 }), json!([{ "a": 1 }])); + select_and_then_compare("$.[?(@ >= 0)]", json!({ "a": 1 }), json!([1])); } #[test] @@ -180,7 +178,7 @@ fn op_eq_for_string_value() { select_and_then_compare( r#"$.[?(@.a == "b")]"#, - json!({ "a": "b" }), + json!([{ "a": "b" }]), json!([{ "a": "b" }]), ); } @@ -191,7 +189,7 @@ fn op_ne_for_string_value() { select_and_then_compare( r#"$.[?(@.a != "c")]"#, - json!({ "a": "b" }), + json!([{ "a": "b" }]), json!([{ "a": "b" }]), ); } @@ -209,7 +207,7 @@ fn op_le_for_string_value() { select_and_then_compare( r#"$.[?(@.a <= "b")]"#, - json!({ "a": "b" }), + json!([{ "a": "b" }]), json!([{ "a": "b" }]), ); } @@ -227,7 +225,7 @@ fn op_ge_for_string_value() { select_and_then_compare( r#"$.[?(@.a >= "b")]"#, - json!({ "a": "b" }), + json!([{ "a": "b" }]), json!([{ "a": "b" }]), ); } @@ -238,7 +236,7 @@ fn op_eq_for_object_value() { select_and_then_compare( r#"$.[?(@.a == @.c)]"#, - json!({"a": { "1": 1 }, "b": { "2": 2 }, "c": { "1": 1 }}), + json!([{"a": { "1": 1 }, "b": { "2": 2 }, "c": { "1": 1 }}]), json!([{"a": { "1": 1 }, "b": { "2": 2 }, "c": { "1": 1 }}]), ); } @@ -249,7 +247,7 @@ fn op_ne_for_object_value() { select_and_then_compare( r#"$.[?(@.a != @.c)]"#, - json!({"a": { "1": 1 }, "b": { "2": 2 }, "c": { "1": 1 }}), + json!([{"a": { "1": 1 }, "b": { "2": 2 }, "c": { "1": 1 }}]), json!([]), ); } @@ -311,7 +309,7 @@ fn op_ne_for_complex_value() { select_and_then_compare( r#"$.[?("1" != @.a)]"#, - json!({ "a": { "b": 1 } }), + json!([{ "a": { "b": 1 } }]), json!([{ "a": { "b": 1 } }]), ); } @@ -352,11 +350,11 @@ fn op_for_same_type() { select_and_then_compare( r#"$..[?(@.a == 1)]"#, - json!({ + json!([{ "a": 1, "b" : {"a": 1}, "c" : {"a": 1} - }), + }]), json!([ { "a": 1, diff --git a/tests/return_type.rs b/tests/return_type.rs index 11c1e1b..a85e2f7 100644 --- a/tests/return_type.rs +++ b/tests/return_type.rs @@ -41,15 +41,13 @@ fn return_type_for_child_object_matched() { setup(); select_and_then_compare( - "$.school[?(@.friends[0])]", + "$.school[?(@[0])]", read_json("./json_examples/data_obj.json"), - json!([{ - "friends": [ + json!([[ {"id": 0, "name": "Millicent Norman"}, {"id": 1, "name": "Vincent Cannon" }, {"id": 2, "name": "Gray Berry"} - ] - }]), + ]]), ); } @@ -71,13 +69,11 @@ fn return_type_for_object_filter_true() { select_and_then_compare( "$.school[?(1==1)]", read_json("./json_examples/data_obj.json"), - json!([{ - "friends": [ + json!([[ {"id": 0, "name": "Millicent Norman"}, {"id": 1, "name": "Vincent Cannon" }, {"id": 2, "name": "Gray Berry"} - ] - }]), + ]]), ); } From d663ec38893d07208eef2799e66c7b6eb570f5c3 Mon Sep 17 00:00:00 2001 From: meir Date: Thu, 8 Sep 2022 16:52:11 +0300 Subject: [PATCH 3/6] format fixes --- src/json_path.rs | 25 ++++---------- tests/filter.rs | 78 ++++++++++++++++++++++---------------------- tests/op.rs | 8 ++--- tests/return_type.rs | 8 ++--- 4 files changed, 53 insertions(+), 66 deletions(-) diff --git a/src/json_path.rs b/src/json_path.rs index aed3a9c..b909991 100644 --- a/src/json_path.rs +++ b/src/json_path.rs @@ -776,12 +776,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { results: Vec::new(), root: calc_data.root, }; - self.calc_internal( - term.into_inner(), - calc_data.root, - None, - &mut new_calc_data, - ); + self.calc_internal(term.into_inner(), calc_data.root, None, &mut new_calc_data); if new_calc_data.results.len() == 1 { TermEvaluationResult::Value(new_calc_data.results.pop().unwrap().res) } else { @@ -884,12 +879,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { Some(curr) => { match curr.as_rule() { Rule::full_scan => { - self.calc_internal( - pairs.clone(), - json, - path_tracker.clone(), - calc_data, - ); + self.calc_internal(pairs.clone(), json, path_tracker.clone(), calc_data); self.calc_full_scan(pairs, json, path_tracker, calc_data); } Rule::all => self.calc_all(pairs, json, path_tracker, calc_data), @@ -904,7 +894,9 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { self.calc_range(pairs, curr, json, path_tracker, calc_data); } Rule::filter => { - if json.get_type() == SelectValueType::Array || json.get_type() == SelectValueType::Object { + if json.get_type() == SelectValueType::Array + || json.get_type() == SelectValueType::Object + { /* lets expend the array, this is how most json path engines work. * Pesonally, I think this if should not exists. */ let values = json.values().unwrap(); @@ -959,12 +951,7 @@ impl<'i, UPTG: UserPathTrackerGenerator> PathCalculator<'i, UPTG> { root: json, }; if self.tracker_generator.is_some() { - self.calc_internal( - root, - json, - Some(create_empty_tracker()), - &mut calc_data, - ); + self.calc_internal(root, json, Some(create_empty_tracker()), &mut calc_data); } else { self.calc_internal(root, json, None, &mut calc_data); } diff --git a/tests/filter.rs b/tests/filter.rs index f4c97c7..fce1d1e 100644 --- a/tests/filter.rs +++ b/tests/filter.rs @@ -327,37 +327,37 @@ fn filter_nested() { select_and_then_compare( "$.store.book[?(@.authors[?(@.lastName == 'Rees')])].title", json!({ - "store": { - "book": [ - { - "authors": [ - { - "firstName": "Nigel", - "lastName": "Rees" - }, - { - "firstName": "Evelyn", - "lastName": "Waugh" - } - ], - "title": "Sayings of the Century" - }, - { - "authors": [ - { - "firstName": "Herman", - "lastName": "Melville" - }, - { - "firstName": "Somebody", - "lastName": "Else" - } - ], - "title": "Moby Dick" - } - ] - } - }), + "store": { + "book": [ + { + "authors": [ + { + "firstName": "Nigel", + "lastName": "Rees" + }, + { + "firstName": "Evelyn", + "lastName": "Waugh" + } + ], + "title": "Sayings of the Century" + }, + { + "authors": [ + { + "firstName": "Herman", + "lastName": "Melville" + }, + { + "firstName": "Somebody", + "lastName": "Else" + } + ], + "title": "Moby Dick" + } + ] + } + }), json!(["Sayings of the Century"]), ); } @@ -369,14 +369,14 @@ fn filter_inner() { select_and_then_compare( "$[?(@.inner.for.inner=='u8')].id", json!( - { - "a": { - "id": "0:4", - "inner": { - "for": {"inner": "u8", "kind": "primitive"} - } - } - }), + { + "a": { + "id": "0:4", + "inner": { + "for": {"inner": "u8", "kind": "primitive"} + } + } + }), json!(["0:4"]), ); } diff --git a/tests/op.rs b/tests/op.rs index 17f430d..90e3b1a 100644 --- a/tests/op.rs +++ b/tests/op.rs @@ -13,10 +13,10 @@ fn op_object_eq() { "$.school[?(@ == @)]", read_json("./json_examples/data_obj.json"), json!([[ - {"id": 0, "name": "Millicent Norman"}, - {"id": 1, "name": "Vincent Cannon" }, - {"id": 2, "name": "Gray Berry"} - ]]), + {"id": 0, "name": "Millicent Norman"}, + {"id": 1, "name": "Vincent Cannon" }, + {"id": 2, "name": "Gray Berry"} + ]]), ); } diff --git a/tests/return_type.rs b/tests/return_type.rs index a85e2f7..430707c 100644 --- a/tests/return_type.rs +++ b/tests/return_type.rs @@ -70,10 +70,10 @@ fn return_type_for_object_filter_true() { "$.school[?(1==1)]", read_json("./json_examples/data_obj.json"), json!([[ - {"id": 0, "name": "Millicent Norman"}, - {"id": 1, "name": "Vincent Cannon" }, - {"id": 2, "name": "Gray Berry"} - ]]), + {"id": 0, "name": "Millicent Norman"}, + {"id": 1, "name": "Vincent Cannon" }, + {"id": 2, "name": "Gray Berry"} + ]]), ); } From 4b2d17589fdbecf14397b26a0c300d0799920c76 Mon Sep 17 00:00:00 2001 From: meir Date: Thu, 15 Sep 2022 08:03:25 +0300 Subject: [PATCH 4/6] improve comment and documentation. --- src/json_path.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/json_path.rs b/src/json_path.rs index b909991..859bb0c 100644 --- a/src/json_path.rs +++ b/src/json_path.rs @@ -15,6 +15,7 @@ pub enum JsonPathToken { Number, } +/* Struct that represent a compiled json path query. */ #[derive(Debug)] pub struct Query<'i> { // query: QueryElement<'i> @@ -30,6 +31,10 @@ pub struct QueryCompilationError { } impl<'i> Query<'i> { + /// Pop the last element from the compiled json path. + /// For example, if the json path is $.foo.bar then pop_last + /// will return bar and leave the json path query with foo only + /// ($.foo) #[allow(dead_code)] pub fn pop_last(&mut self) -> Option<(String, JsonPathToken)> { let last = self.root.next_back(); @@ -57,6 +62,8 @@ impl<'i> Query<'i> { } } + /// Returns the amount of elements in the json path + /// Example: $.foo.bar have 2 elements #[allow(dead_code)] pub fn size(&mut self) -> usize { if self.size.is_some() { @@ -66,6 +73,11 @@ impl<'i> Query<'i> { self.size() } + /// Results whether or not the compiled json path is static + /// Static path is a path that is promised to have at most a single result. + /// Example: + /// static path: $.foo.bar + /// none static path: $.*.bar #[allow(dead_code)] pub fn is_static(&mut self) -> bool { if self.is_static.is_some() { @@ -119,6 +131,8 @@ impl std::fmt::Display for Rule { } } +/// Compire the given string query into a query object. +/// Returns error on compilation error. pub(crate) fn compile(path: &str) -> Result { let query = JsonPathParser::parse(Rule::query, path); match query { @@ -201,6 +215,7 @@ pub trait UserPathTrackerGenerator { fn generate(&self) -> Self::PT; } +/* Dummy path tracker, indicating that there is no need to track results paths. */ pub struct DummyTracker; impl UserPathTracker for DummyTracker { fn add_str(&mut self, _s: &str) {} @@ -210,6 +225,7 @@ impl UserPathTracker for DummyTracker { } } +/* A dummy path tracker generator, indicating that there is no need to track results paths. */ pub struct DummyTrackerGenerator; impl UserPathTrackerGenerator for DummyTrackerGenerator { type PT = DummyTracker; @@ -224,6 +240,7 @@ pub enum PTrackerElement { Index(usize), } +/* An actual representation of a path that the user get as a result. */ #[derive(Debug, PartialEq)] pub struct PTracker { pub elemenets: Vec, @@ -248,6 +265,7 @@ impl UserPathTracker for PTracker { } } +/* Used to generate paths trackers. */ pub struct PTrackerGenerator; impl UserPathTrackerGenerator for PTrackerGenerator { type PT = PTracker; @@ -265,6 +283,12 @@ enum PathTrackerElement<'i> { Root, } +/* Struct that used to track paths of query results. + * This struct is used to hold the path that lead to the + * current location (when calculating the json path). + * Once we have a match we can run (in a reverse order) + * on the path tracker and add the path to the result as + * a PTracker object. */ #[derive(Clone)] struct PathTracker<'i, 'j> { parent: Option<&'j PathTracker<'i, 'j>>, @@ -298,6 +322,7 @@ const fn create_index_tracker<'i, 'j>( } } +/* Enum for filter results */ enum TermEvaluationResult<'i, 'j, S: SelectValue> { Integer(i64), Float(f64), @@ -413,6 +438,9 @@ impl<'i, 'j, S: SelectValue> TermEvaluationResult<'i, 'j, S> { } } +/* This struct is used to calculate a json path on a json object. + * The struct contains the query and the tracker generator that allows to create + * path tracker to tracker paths that lead to different results. */ #[derive(Debug)] pub struct PathCalculator<'i, UPTG: UserPathTrackerGenerator> { pub query: Option<&'i Query<'i>>, From 9b12720314da80728b0f528b55dc81487404478d Mon Sep 17 00:00:00 2001 From: "Meir Shpilraien (Spielrein)" Date: Thu, 15 Sep 2022 14:34:12 +0300 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Omer Shadmi <76992134+oshadmi@users.noreply.github.com> --- src/json_path.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/json_path.rs b/src/json_path.rs index 859bb0c..bdf5105 100644 --- a/src/json_path.rs +++ b/src/json_path.rs @@ -63,7 +63,7 @@ impl<'i> Query<'i> { } /// Returns the amount of elements in the json path - /// Example: $.foo.bar have 2 elements + /// Example: $.foo.bar has 2 elements #[allow(dead_code)] pub fn size(&mut self) -> usize { if self.size.is_some() { @@ -131,7 +131,7 @@ impl std::fmt::Display for Rule { } } -/// Compire the given string query into a query object. +/// Compile the given string query into a query object. /// Returns error on compilation error. pub(crate) fn compile(path: &str) -> Result { let query = JsonPathParser::parse(Rule::query, path); @@ -240,7 +240,7 @@ pub enum PTrackerElement { Index(usize), } -/* An actual representation of a path that the user get as a result. */ +/* An actual representation of a path that the user gets as a result. */ #[derive(Debug, PartialEq)] pub struct PTracker { pub elemenets: Vec, From 87d1cc8cdca8dcf95eef6f3fbdca78a8f589b553 Mon Sep 17 00:00:00 2001 From: meir Date: Thu, 15 Sep 2022 14:36:44 +0300 Subject: [PATCH 6/6] review fixes --- tests/filter.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/filter.rs b/tests/filter.rs index fce1d1e..30e58ae 100644 --- a/tests/filter.rs +++ b/tests/filter.rs @@ -101,6 +101,18 @@ fn filter_parent_with_matched_child() { } ]), ); + + select_and_then_compare( + "$.a[?(@.b.c == 1)]", + json!({ + "a": { + "b": { + "c": 1 + } + } + }), + json!([]), + ); } #[test]