stretchr / objx

Go package for dealing with maps, slices, JSON and other data.
MIT License
691 stars 75 forks source link

Set operator with not expected behavior #97

Closed PeerXu closed 2 years ago

PeerXu commented 4 years ago

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 commented 4 years ago

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

Digl commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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