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

Set operator with not expected behavior #97

Closed
PeerXu opened this issue May 12, 2020 · 5 comments · Fixed by #105
Closed

Set operator with not expected behavior #97

PeerXu opened this issue May 12, 2020 · 5 comments · Fixed by #105
Labels
Milestone

Comments

@PeerXu
Copy link

PeerXu commented May 12, 2020

create map by FromJSON or New(with map[string]interface{}), and Set with fields got the difference result.

m := objx.FromJSON(`{"a": {"b": 1}}`)
m.Set("a.c", 2)
fmt.Println(m.JSON())  // output: {"a": {"c": 2}}
m := objx.New(map[string]interface{}{
  "a": map[string]interface{}{
    "b": 1,
  },
})
m.Set("a.c", 2)
fmt.Println(m.JSON())  // output: {"a": {"b": 1, "c": 2}}
@asturm-fino
Copy link

I stumbled upon a very similar, related bug where adding a previously non-existing key-value pair to a nested map with the Set-method resulted in the siblings of the new key-value pair being deleted from the map.

As i dug into the code i found the culprit:
https://github.com/stretchr/objx/blob/master/accessors.go#L136-L139

_, ok := curMSI[thisSel].(map[string]interface{})
if (curMSI[thisSel] == nil || !ok) && index == -1 && isSet {
	curMSI[thisSel] = map[string]interface{}{}
}

If the object at thisSel is not a map, you basically delete all the existing data.
In @PeerXu 's example, this is likely due to objx.FromJSON returning the type not as a map[string]interface{}
So the type assertion at key a fails and the old data is dropped.
The input c: 2 is then written into the fresh, empty map and the result is returned.

So the bug here is the assumption that there will only be data of type map[string]interface{}
A solution must take into account that there can possibly be a variety of map-like types, including

  • objx.Map (this was what caused this error in my code)
  • bson.M
  • ....

@hanzei hanzei added this to the v1.0 milestone Jul 16, 2021
@hanzei hanzei added the bug label Jul 16, 2021
@Digl
Copy link

Digl commented Mar 8, 2022

Hi!
This bug still exists (I think).

test, _ := objx.FromJSON(`{ "root": { "other": "value", "test": [ { "name": "test", "name2": "test2" } ] } }`)
test2, _ := objx.FromJSON(`{"name": "new test", "name2": "new test2"}`)
test.Set("root.test[0]", test2)

result: {"root":{"test":{"name":"new test","name2":"new test2"}}}
expected: { "root": { "other": "value", "test": [ {"name": "new test", "name2": "new test2"} ] } }

root.other is completely dropped...

Am I doing something wrong?

@Digl
Copy link

Digl commented Mar 8, 2022

this is also the case when doing something like this:

slice := data.Get("root.test").ObjxMapSlice()
temp, _ := slice.JSON()
//change some fields (e.g. string replace)
newSlice, _ := objx.FromJSON(temp)
data.Set("root.test", newSlice)

newSlice will be added to a new empty objx.Map
everything else will be lost

@Digl
Copy link

Digl commented Mar 8, 2022

oh I just saw that 0.3.0 is the latest release and it is from july 2020, will there be a new release?

@Digl
Copy link

Digl commented Mar 8, 2022

fixed it with:
go get -u github.com/stretchr/objx@27373ced094756c353f68145eb313a575ca613ed

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

Successfully merging a pull request may close this issue.

4 participants