Merge pull request #50 from hiltontj/hiltontj/49-false--positives-fro…

…m-filters [#49] Fix bug producing false positives in filters
hiltontj · Jul 13, 2023 · 9b6e334 · 9b6e334
2 parents 04c6ccf + 9af60b9
commit 9b6e334
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 12 deletions.
diff --git a/serde_json_path/CHANGELOG.md b/serde_json_path/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 # Unreleased
 
+* **fixed**: Fixed an issue in the evaluation of `SingularQuery`s that was producing false positive query results when relative singular queries, e.g., `@.bar`, were being used as comparables in a filter, e.g., `$.foo[?(@.bar == 'baz')]` [#50]
+
+[#50]: https://github.com/hiltontj/serde_json_path/pull/50
+
 # 0.6.1 (5 July 2023)
 
 * **documentation**: Updated links to JSONPath specification to latest version (base 14) [#43]

diff --git a/serde_json_path/tests/regressions.rs b/serde_json_path/tests/regressions.rs
@@ -0,0 +1,13 @@
+use serde_json::json;
+use serde_json_path::JsonPath;
+#[cfg(feature = "trace")]
+use test_log::test;
+
+// This test is meant for issue #49, which can be found here:
+// https://github.com/hiltontj/serde_json_path/issues/49
+#[test]
+fn issue_49() {
+    let value = json!({"a": 1, "b": 2});
+    let path = JsonPath::parse("$[?(@.a == 2)]").expect("parses JSONPath");
+    assert!(path.query(&value).is_empty());
+}
diff --git a/serde_json_path_core/CHANGELOG.md b/serde_json_path_core/CHANGELOG.md
@@ -7,5 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 # Unreleased
 
+* **fixed**: Fixed an issue in the evaluation of `SingularQuery`s that was producing false positive query results when relative singular queries, e.g., `@.bar`, were being used as comparables in a filter, e.g., `$.foo[?(@.bar == 'baz')]` [#50]
+
+[#50]: https://github.com/hiltontj/serde_json_path/pull/50
+
+# 0.1.0 (2 April 2023)
+
 Initial Release
 
diff --git a/serde_json_path_core/src/spec/selector/filter.rs b/serde_json_path_core/src/spec/selector/filter.rs
@@ -366,6 +366,7 @@ impl std::fmt::Display for Comparable {
 
 impl Comparable {
     #[doc(hidden)]
+    #[cfg_attr(feature = "trace", tracing::instrument(name = "Comparable::as_value", level = "trace", parent = None, ret))]
     pub fn as_value<'a, 'b: 'a>(
         &'a self,
         current: &'b Value,
@@ -493,6 +494,7 @@ pub struct SingularQuery {
 
 impl SingularQuery {
     /// Evaluate the singular query
+    #[cfg_attr(feature = "trace", tracing::instrument(name = "SingularQuery::eval_query", level = "trace", parent = None, ret))]
     pub fn eval_query<'b>(&self, current: &'b Value, root: &'b Value) -> Option<&'b Value> {
         let mut target = match self.kind {
             SingularQueryKind::Absolute => root,
@@ -501,21 +503,20 @@ impl SingularQuery {
         for segment in &self.segments {
             match segment {
                 SingularQuerySegment::Name(name) => {
-                    if let Some(v) = target.as_object() {
-                        if let Some(t) = v.get(name.as_str()) {
-                            target = t;
-                        } else {
-                            return None;
-                        }
+                    if let Some(t) = target.as_object().and_then(|o| o.get(name.as_str())) {
+                        target = t;
+                    } else {
+                        return None;
                     }
                 }
                 SingularQuerySegment::Index(index) => {
-                    if let Some(l) = target.as_array() {
-                        if let Some(t) = usize::try_from(index.0).ok().and_then(|i| l.get(i)) {
-                            target = t;
-                        } else {
-                            return None;
-                        }
+                    if let Some(t) = target
+                        .as_array()
+                        .and_then(|l| usize::try_from(index.0).ok().and_then(|i| l.get(i)))
+                    {
+                        target = t;
+                    } else {
+                        return None;
                     }
                 }
             }