Fix decoded maps when unmarshaling fixtures from a YAML file #23
Conversation
As shown in #22, when we try to unmarshal fixtures from a YAML file, we end up with a crash in the generator due to a type mismatch. This turns out to be because of [1]. Because YAML technically supports boolean keys, the YAML library unmarshals to maps in a way to handle them (`map[interface{}]interface{}`), even if it's not suitable for most users (the issue strongly suggests that a lot of people would like `map[string]interface{}`). Here we work around the problem by post-processing fixtures that we've loaded via YAML by recursing through and changing all instances of `map[interface{}]interface{}` to `map[string]interface{}`. I've added a few basic tests, but also tested that the call reported in #22 now works as expected. Fixes #22. [1] go-yaml/yaml#139
| context, schema, example.value)) | ||
| panic(fmt.Sprintf( | ||
| "%sSchema is an object:\n%s\n\nBut example is (type: %v):\n%s", | ||
| context, schema, reflect.TypeOf(example.value), example.value)) |
There was a problem hiding this comment.
Also added a type hint here to help with debugging in the future (it's how I found this problem).
mapstr.go
Outdated
| func stringifyKeysInterfaceMap(in map[interface{}]interface{}) map[string]interface{} { | ||
| res := make(map[string]interface{}) | ||
| for k, v := range in { | ||
| res[fmt.Sprintf("%v", k)] = stringifyKeysMapValue(v) |
There was a problem hiding this comment.
I think we should check that the key is already a string and raise an error if it isn't. Otherwise, this can lead to some really strange behavior. Suppose the original YAML file looks like:
in: a
on: b
under: c
The YAML parser will interpret in and under as strings but it will interpret on as the boolean true. So the output of stringifyKeysMapValue will be:
map[string]interface{}{
"in": "a",
"true": "b",
"under": "c",
}
which is super confusing and almost certainly not what was intended.
There was a problem hiding this comment.
Great point. I adapted this code from elsewhere, but yep, in our case I think it makes perfect sense to be as loud as possible about this problem because we expect it to never happen.
Patched in 24a2bb4. Thanks for taking a look.
|
Thanks for tracking down that bug! I left a comment suggesting a slightly different behavior, but otherwise it looks good. |
We never expect to have a non-string key and this is an internal library so it makes sense in our case to be as loud as possible about this sort of problem.
|
LGTM, thanks! |
As shown in #22, when we try to unmarshal fixtures from a YAML file, we
end up with a crash in the generator due to a type mismatch.
This turns out to be because of [1]. Because YAML technically supports
boolean keys, the YAML library unmarshals to maps in a way to handle
them (
map[interface{}]interface{}), even if it's not suitable for mostusers (the issue strongly suggests that a lot of people would like
map[string]interface{}).Here we work around the problem by post-processing fixtures that we've
loaded via YAML by recursing through and changing all instances of
map[interface{}]interface{}tomap[string]interface{}. I've added afew basic tests, but also tested that the call reported in #22 now works
as expected.
Fixes #22.
[1] go-yaml/yaml#139
r? @tmaxwell-stripe