Handle invalid json paths

[theredfish]

Context

After reading some bugfixes and playing around with edge cases, I think it would be great if the library could handle an invalid json path as an error instead of returning an empty array. This would avoid some bugs or confusing results I think. The specs recommend to return an empty array or false, but I think this is because of the lack of types at this moment (javascript / php). I think the version by the Internet engineering task force is the most recent one but it's globally the same as the initial version.

An alternative for jsonPath to return false in case of no match may be to return an empty array in future. [This is already done in the above.]

Here "no match" seems to cover both empty results and non-existent paths. It leaves a gray area on the implementation details.

Example

Let's take this Value as the data source:

let json_obj = json!({
      "shop": {
        "orders": [
          { "id": 1, "active": true },
          { "id": 2 },
          { "id": 3 },
          { "id": 4, "active": true }
        ],
        "past_orders": [ "AB01", "AB02" ],
        "returned_orders": []
      }
    });

length

Note: I don't know if the last fixes for length function have been released or not. So maybe some of the results here are already solved.

We can't differentiate an empty result from an non-existent path. Both json paths will return Array [ Number(0) ]

1. $.shop.inexistant[*].length()
2. $.shop.returned_orders[*].length()

I would expect an Err(JsonPathNotFound) instead for 1. (or similar).

false positives/negatives

It's more specific to the implementation I'm doing with Grillon but it's also the case for native assertions with the Rust test lib. I can't tell an empty result from a non-inexistent path.

If we keep the initial data source, it's all good. Let's say we want to verify AB01 and AB02 are not in returned_orders, the assertions passes :

let path = "$.shop.returned_orders[*]";
let val = json_obj.path(path).unwrap();
let should_not_be = json!(["AB01", "AB02"]);
assert_ne!(val, should_not_be);

println!(" val: {val:#?} \n should not be: {should_not_be:#?}");

warning: `jpath-test` (bin "jpath-test") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.86s
     Running `target/debug/jpath-test`
 val: Array [] 
 should not be: Array [
    String("AB01"),
    String("AB02"),
]

But now let's remove returned_orders from the data, the result is the following:

warning: `jpath-test` (bin "jpath-test") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.90s
     Running `target/debug/jpath-test`
 val: Array [] 
 should not be: Array [
    String("AB01"),
    String("AB02"),
]

We observe a false positive. We think there is no issue but in reality the path doesn't exist, the result isn't empty, the data doesn't contain what we're looking at. Instead I would like to handle it as an Error and produce meaningful information to my users. Like raising a warning, since it's not completely false, you don't have any returned orders but the fact that it's missing from the result shouldn't be ignored.

Conclusion

I don't think I have a way to make distinction right now. I also think it causes some bugs in the interpretation of results in the library and by users. The advantage of a strongly typed language is that we can take advantage on the type system to return meaningful information. I wonder if it would be possible to handle non-existent json paths in the library? Do we have any cases where handling an error could prevent basic functionality from working? If not, can we work on a new version?

[besok]

Well. That makes sense. As a matter of fact, the same intention appeared in an attempt(#25) to handle the length operator in that way. I don't mind to propagate it to the whole library.

Also, I would not consider the case when the path is not found as an error. Instead, I would distinguish the result by wrapping it to an enum like

  enum QueryResult{
     Found(JsonPathValue<...>),
     NotFound,  
  }

I will be able to tackle it next week

[theredfish]

Do you think you will have more cases than those two for the QueryResult? Because in that case an Option could be good enough.

[besok]

Indeed, a fair point. I thought, perhaps, it can get bigger or obtain some extra information like

   enum QueryResult{
     Found(JsonPathValue<...>),
     NotFound(Reason),   
  }

but probably better to stick with Option for now

[besok]

Distinguish empty and no results

[besok]

So, I extended the enum JsonPathValue with a NoValue and the method find_slice then can return it wrapped with a Vec. Nevetheless, I did not change the contract of the major method find but instead it is supplemented with the returned Json::Null when there is no match , otherwise it will be array of values including empty array.

[theredfish]

Thanks @besok ! Will check that soon!