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

SetP has unexpected outcome with gabs.Container as value #108

Open
2manyvcos opened this issue Aug 27, 2021 · 1 comment
Open

SetP has unexpected outcome with gabs.Container as value #108

2manyvcos opened this issue Aug 27, 2021 · 1 comment

Comments

@2manyvcos
Copy link

Please look at the following example:

package main

import (
	"fmt"
	
	"github.com/Jeffail/gabs/v2"
)

func main() {
	a := gabs.New()
	
        b := gabs.New()
        b.SetP(1, "test")

        // normally: a.SetP(b.Data(), "inner")
        a.SetP(b, "inner")
	
	fmt.Println(a.Path("inner.test").Data())
	fmt.Println(a.String())
}

Output:

<nil>
{"inner":{"test":1}}

As you can see, with gabs v2 there is an issue when setting an attribute to a *gabs.Container instead of its Data().

When you do a.SetP(b, "inner") with gabs v1, "inner" would result in an empty object, which is fine as it indicates that you are not allowed to do this and should do a.SetP(b.Data(), "inner") instead.

However, with v2 when you do a.SetP(b, "inner"), a.String() returns a valid JSON as if the operation was successful and so a.inner looks like it was set to the contents of b.
But subsequent operations on "inner" would fail (e.g. a.SetP(2, "inner.test2")).
When merging a into another gabs object, inner is also replaced as a whole instead of merging its contents.

I think checking if the value of SetP (and potentially other related functions) is a gabs.Container would be a really helpful thing to prevent this confusion, for instance:

func (g *Container) SetP(value interface{}, path string) (*Container, error) {
	if checkIsContainer(value) {
		return g.Set(value.Data(), DotPathToSlice(path)...)
	}
	return g.Set(value, DotPathToSlice(path)...)
}

func checkIsContainer(v interface{}) bool {
	_, ok := v.(*Container)
	return ok
}
@flowchartsman
Copy link
Contributor

This is the same underlying cause as #114, namely that the value you're creating is a *gabs.Container and not a map[string]interface{}, so S | Search | Path does not know how to traverse its inner structure. What's called for is probably special casing in the Set code, similar to #121, though I think a more general approach is probably called for like I discuss in #114.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants