ohler55 / ojg

Optimized JSON for Go
MIT License
857 stars 49 forks source link

`jp` maches array elements in reverse order #23

Closed setrofim closed 4 years ago

setrofim commented 4 years ago

When a JSONPath query uses wildcard or union indexing (.e.g $.array[*] or $.array[0,1]), array elements appear to be matched in reverse order. Here is a minimal reproducer:

bash$ /bin/cat bug_test.go 
package test

import (
        "testing"

        "github.com/stretchr/testify/assert"
        "github.com/ohler55/ojg/jp"
        "github.com/ohler55/ojg/oj"
)

func TestCase(t *testing.T) {
    obj, _ := oj.ParseString(`{
        "a":[
            {"x":1,"y":2,"z":3},
            {"x":2,"y":4,"z":6}
        ]
    }
    `)

    x, _ := jp.ParseString("$.a[*].y")
    ys := x.Get(obj)
    assert.Equal(t, []interface{}{2, 4}, ys)
}

bash$ go test
--- FAIL: TestCase (0.00s)
    bug_test.go:22: 
                Error Trace:    bug_test.go:22
                Error:          Not equal: 
                                expected: []interface {}{2, 4}
                                actual  : []interface {}{4, 2}

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1,4 +1,4 @@
                                 ([]interface {}) (len=2) {
                                - (int) 2,
                                - (int) 4
                                + (int64) 4,
                                + (int64) 2
                                 }
                Test:           TestCase
FAIL
exit status 1
FAIL    _/mnt/Data/work/code/jp-bug     0.002s
ohler55 commented 4 years ago

I'm not sure the order needs to be preserved in wildcard searches but I'll take a look at flipping the order.

setrofim commented 4 years ago

Great! Thank you for the quick response, and for looking into this!

It is true that the JSONPath "spec" does not explicitly state this. However, it suggests that the behaviour is meant to be equivalent to that of the XPath wildcard, and the XPath spec explicitly states that the order is preserved: https://www.w3.org/TR/2017/REC-xpath-31-20170321/#id-unary-lookup (See bullet 4. for arrays).

Also, FWIW, other implementations (e.g. https://jsonpath.com/) seem to preserve order.

ohler55 commented 4 years ago

Working on it.

ohler55 commented 4 years ago

The jp-order branch has the fixes. It fixes union, slice, and wildcard. Let me know if you find anything off. If not I'll merge and release.

setrofim commented 4 years ago

Yes, I can confirm that, with jp-order, it now works as expected. Thank you for the fix!

ohler55 commented 4 years ago

Release v1.2.1 has the fixes.

setrofim commented 4 years ago

Thanks again for the quick turn-around!

ohler55 commented 4 years ago

Slower than I like but glad it is fixed.