Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSONPath includes on collection with single entry does not work #1469

Closed
janwytze opened this issue Apr 20, 2023 · 5 comments · Fixed by #1522
Closed

JSONPath includes on collection with single entry does not work #1469

janwytze opened this issue Apr 20, 2023 · 5 comments · Fixed by #1522
Assignees
Labels
bug Something isn't working
Milestone

Comments

@janwytze
Copy link

What is the current bug behavior?

Using the includes assertion on a collection with a single item does not work.

Steps to reproduce

When I run the assertion jsonpath "$[*].id" includes 4 on the following response is will return an error:

[
  {
    "id": 4,
    "name": "name"
  }
]

The error:

jsonpath "$[*].id" includes 4
  actual:   int <4>
  expected: includes int <4>
  >>> types between actual and expected are not consistent

But when there are multiple entries the assertion does not fail.

What is the expected correct behavior?

The above assertion should be true, even on a single item

Execution context

  • Hurl Version (hurl --version):
hurl 2.0.1 libcurl/7.81.0 OpenSSL/3.0.2 zlib/1.2.11 brotli/1.0.9 zstd/1.4.8 libidn2/2.3.2 libssh/0.9.6/openssl/zlib nghttp2/1.43.0
Features (libcurl):  alt-svc AsynchDNS brotli HSTS HTTP2 IDN IPv6 Largefile libz NTLM NTLM_WB SPNEGO SSL TLS-SRP UnixSockets zstd
Features (built-in): brotli
  • Operating system and version: Ubuntu 22.04

P.S. Is there a way to use and/or with assertions? Then I could use jsonpath "$[*].id" includes 4 || jsonpath "$[*].id" == 4

@janwytze janwytze added the bug Something isn't working label Apr 20, 2023
@fabricereix
Copy link
Collaborator

fabricereix commented Apr 20, 2023

Hi @janwytze,

In fact, the issue comes from the fact that the jsonpath returns a single value.

Standard jsonpath always returns a collection, which most of the time is not meaningful, and harder to test.
That's the reason why we coerced one-item collections to single items.
But in your case, it will indeed make more sense to keep the collection and have your test working.

Here are a few examples for ilustration.

Here are 2 inputs

[
  {
    "id": 1,
    "name": "car"
  }
]
[
  {
    "id": 1,
    "name": "car"
  },
  {
    "id": 2,
    "name": "bike"
  }
]

For the jsonpath $[0].id
jsonpath.com returns [1] for the 2 inputs
Hurl returns 1 for the 2 inputs

For the jsonpath $[*].id
jsonpath.com returns [1] and [1,2]
Hurl returns 1 and [1,2] (currently)
Hurl should return [1] and [1,2] (fix)

@jcamiel
Copy link
Collaborator

jcamiel commented Apr 20, 2023

@fabricereix with one of the most supported Java library https://github.com/json-path/JsonPath, we have this result:

import com.jayway.jsonpath.Configuration
import com.jayway.jsonpath.JsonPath

fun main() {

    val json = """
        [
          {
            "id": 1,
            "name": "car"
          }
        ]
    """.trimIndent()

    val document = Configuration.defaultConfiguration().jsonProvider().parse(json)

    val resultA: Int = JsonPath.read(document, "$[0].id");
    assert(resultA == 1)

    val resultB: List<Int> = JsonPath.read(document, "$[*].id");
    assert(resultB == listOf(1))
}

which corresponds well to the defined the target behaviour in your response (different from the current Hurl implementation).

Note:

val resultB: Int = JsonPath.read(document, "$[*].id");

Compiles but triggers a runtime exception.

@fabricereix fabricereix self-assigned this Apr 24, 2023
@jcamiel jcamiel modified the milestone: 3.0.0 Apr 24, 2023
@zachelrath
Copy link

@fabricereix One of the guys on our team also ran into this and lost an hour trying to figure out why hurl's JSONPath implementation was not returning the standard syntax.

I would urge the Hurl team to consider changing this behavior in an upcoming major release to make Hurl's JSONPath as standards-compliant and intuitive as possible. The decision to coerce single-entry arrays to an object is very problematic. I have seen many products / tools do this "as a convenience" and it always turns out to cause problems. The big issue is that the data set you're querying against is not always deterministic ---> if a query might return 1 record but sometimes returns 2+, then it's impossible to use Hurl to reliably test this, because you can't even assert on the count, you have to do some sort of crazy "duck-typing" to check whether the result of the jsonpath is an object or an array before running any further assertions, which is way more cumbersome and confusing than simply doing a [0] on the result of the array.

Having a consistent, standards-compliant API is far better than trying to save developers from having to enter the 3 characters [0] to get the first item of an array.

@fabricereix
Copy link
Collaborator

yes, as explained in the previous comments, we will remove this single-entry coercion.

@jcamiel
Copy link
Collaborator

jcamiel commented Apr 27, 2023

@zachelrath we have choose a compromise between being strictly compliant to the spec https://goessner.net/articles/JsonPath/ and introducing some convenience. We have looked at various implementations of JSONPath in different languages, and you can find the same behaviour :

Regarding this JSON:

{
  "welcome": {
    "message": ["Hello World!"]
  }
}

And the following JSONPath query: $.welcome.message[0]:

In the documentation:

When using JsonPath in java its important to know what type you expect in your result. JsonPath will automatically try to cast the result to the type expected by the invoker.

//Will throw an java.lang.ClassCastException    
List<String> list = JsonPath.parse(json).read("$.store.book[0].author")

//Works fine
String author = JsonPath.parse(json).read("$.store.book[0].author")

Since WireMock’s matching operators all work on strings, the value selected by the JSONPath expression
will be coerced to a string before the match is evaluated. This true even if the returned value is an
object or array.

I'm sure you can find various library that return an array, regarding of the queries, including https://goessner.net/articles/JsonPath/ and https://jsonpath.com. But, you can agree that Hurl is not the only tool/package to return the "natural" value of a query ("natural" is subjective of course). I'm just saying that what you find not a good choice, can been also seen as a good/pragmatic choice by other (including us!).

Currently, what we've implemented:

Given:

{
  "welcome": {
    "message": ["Hello World!"]
  }
}

$.welcome.message[0] returns a string "Hello World!"
$.welcome.message[*] returns a string "Hello World!"

What we're going to change:

$.welcome.message[0] returns a string "Hello World!"
$.welcome.message[*] returns a list of 1 element ["Hello World!"]

Regarding duck typing, we don't support schemas for the moment but we'll add JSON Schema in the future (see #543). You have also a very basic type assertions with predicates: isBoolean, isString, isCollection, isInteger, isFloat => https://hurl.dev/docs/asserting-response.html#predicates. Maybe that can help to strengthen your tests, with a proper schemas check in the future.

That say, it also always good to discuss with some JSON samples and query. Would you be able to give us some sample that have been problematic?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants