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

entity_unmarshal: Check if type is struct before calling NumField() #113

Open
wants to merge 1 commit into
base: entities
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions entity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,10 @@ var sampleEntityJSON = `{
"latitude": 38.9243983751914,
"longitude": -77.2178385786886
},
"googleAttributes": {
"wi_fi": ["free_wi_fi"],
"welcomes_dogs": ["true"]
},
"meta": {
"accountId": "3549951188342570541",
"uid": "b3JxON",
Expand Down
44 changes: 23 additions & 21 deletions entity_unmarshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,32 +7,34 @@ import (
)

func unmarshal(i interface{}, m map[string]interface{}) interface{} {
var jsonTagToKey = map[string]string{}
val := Indirect(reflect.ValueOf(i))
for i := 0; i < val.Type().NumField(); i++ {
field := val.Type().Field(i)
tag := strings.Replace(field.Tag.Get("json"), ",omitempty", "", -1)
jsonTagToKey[tag] = field.Name
}
if val.Kind() == reflect.Struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this if statement should probably wrap everything i think in this function, because the part you wrapped generates jsonTagToKey and that part is used in the second part (so if we don't actually populate jsonTagToKey there's no use in doing this part.

Do you have an example for which this was breaking? Should we add something to the entity_test.go file?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes that makes sense--I've updated it! I've also added google attributes to the test. prior to my fix, adding google attributes resulted in a panic when trying to run the tests

var jsonTagToKey = map[string]string{}
for i := 0; i < val.Type().NumField(); i++ {
field := val.Type().Field(i)
tag := strings.Replace(field.Tag.Get("json"), ",omitempty", "", -1)
jsonTagToKey[tag] = field.Name
}

for tag, val := range m {
if _, ok := jsonTagToKey[tag]; ok {
if val == nil {
v := Indirect(reflect.ValueOf(i)).FieldByName(jsonTagToKey[tag])
for tag, val := range m {
if _, ok := jsonTagToKey[tag]; ok {
if val == nil {
v := Indirect(reflect.ValueOf(i)).FieldByName(jsonTagToKey[tag])

// Check if double pointer
if v.Type().Kind() == reflect.Ptr {
t := v.Type()
for t.Kind() == reflect.Ptr && t.Elem().Kind() == reflect.Ptr {
t = t.Elem()
// Check if double pointer
if v.Type().Kind() == reflect.Ptr {
t := v.Type()
for t.Kind() == reflect.Ptr && t.Elem().Kind() == reflect.Ptr {
t = t.Elem()
}
typedNil := reflect.New(t)
Indirect(reflect.ValueOf(i)).FieldByName(jsonTagToKey[tag]).Set(reflect.ValueOf(typedNil.Interface()))
}
typedNil := reflect.New(t)
Indirect(reflect.ValueOf(i)).FieldByName(jsonTagToKey[tag]).Set(reflect.ValueOf(typedNil.Interface()))
} else if vMap, ok := val.(map[string]interface{}); ok {
v := Indirect(reflect.ValueOf(i)).FieldByName(jsonTagToKey[tag])
r := unmarshal(v.Interface(), vMap)
Indirect(reflect.ValueOf(i)).FieldByName(jsonTagToKey[tag]).Set(reflect.ValueOf(r))
}
} else if vMap, ok := val.(map[string]interface{}); ok {
v := Indirect(reflect.ValueOf(i)).FieldByName(jsonTagToKey[tag])
r := unmarshal(v.Interface(), vMap)
Indirect(reflect.ValueOf(i)).FieldByName(jsonTagToKey[tag]).Set(reflect.ValueOf(r))
}
}
}
Expand Down