ohler55 / ojg

Optimized JSON for Go
MIT License
839 stars 50 forks source link

preserve order of JSONPath elements (when using wildcard) #110

Closed thiagodpf closed 1 year ago

thiagodpf commented 1 year ago

Hello Mr Ohler! first i would like to thank you for your amazing work on this parser, my team is very surprised with all the advantages we got by adopting this lib!

I would like to comment that we observed a point in the code where the order of the elements (in an array) is not being preserved, but I cannot say whether this behavior is correct or unexpected.

I saw that a few days ago you published a new release (v1.17.3) with a similar fix, but correcting the Filter, so I thought it was worth opening a ticket to comment on the same problem in Wildcard.

The section that seems unexpected to me is in file jp/get.go, on line 206 (tag v1.17.3). There, the x.reflectGetWild(tv) function is called and returns an inverted list of elements (inside it there is a comment saying that this is the desired behavior). It seems to me that that function is correct, it just needs to change the direction that the loop of the 206 line is traversed, in a similar way to the fix made in the Filter case.

Does what I'm proposing make sense?

ohler55 commented 1 year ago

Items are placed on the stack in reverse order because they are popped in reverse order from how they are pushed. That being said, if the order is not correct it should be fixed so I'll take a look at it tonight.

thiagodpf commented 1 year ago

Thanks for the quick response and attention!

The code below (or on the playground) demonstrates my question in a practical way:

package main

import (
    "fmt"

    "github.com/ohler55/ojg/jp"
)

type Root struct {
    Elements []Element
}

type Element struct {
    Value string
}

func main() {
    data := Root{
        Elements: []Element{
            {Value: "el_1"},
            {Value: "el_2"},
            {Value: "el_3"},
        },
    }

    fmt.Println("Original data:")
    for i, el := range data.Elements {
        fmt.Println("index:", i, "value:", el.Value)
    }

    path, _ := jp.ParseString("$.elements[*]")
    var reversed = path.Get(data)

    fmt.Println("Reversed data:")
    for i, el := range reversed {
        fmt.Println("index:", i, "value:", el.(Element).Value)
    }
}

Here is the console output:

Original data:
index: 0 value: el_1
index: 1 value: el_2
index: 2 value: el_3
Reversed data:
index: 0 value: el_3
index: 1 value: el_2
index: 2 value: el_1
ohler55 commented 1 year ago

Thanks for the test case.

ohler55 commented 1 year ago

Please try the wild-reflect-order branch.

thiagodpf commented 1 year ago

It worked perfectly in the real scenarios I have with me, both with slices of structs and pointers! ✅🟢 (I think) it's valid to promote this branch in a new release 🚀

I would like to thank you very much for the speed with which you resolved the issue. Thanks so much for sharing your knowledge and time with the community. I leave here my sincere feeling of gratitude!

ohler55 commented 1 year ago

Released v1.17.4!