From 1a039393f56292ab77d142a695fa17bfe6be094a Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 27 May 2020 13:02:22 -0400 Subject: [PATCH] config: add HookTranslteKeys This hook replaces lib.TranslateKeys and has a number of advantages: 1. Primarily, aliases for fields are defined on the field itself, making the aliases much easier to maintain, and more obvious to the reader. 2. TranslateKeys translation rules are not aware of structure. It could very easily incorrectly translate a key on one struct that was intended to be a translation rule for a completely different struct, leading to very hard to debug errors. The hook removes the need for the unexpected "translation rule is an empty string to indicate stop traversal" special case. 3. TranslateKeys attempts to duplicate a bunch of tree traversal logic that already exists in mapstructure. Using mapstructure for traversal removes the need to traverse the entire structure multiple times, and makes the behaviour more obvious to the reader. This change is being made to enable a future change of replacing PatchSliceOfMaps. TranslateKeys sits in between PatchSliceOfMaps and mapstructure.Decode, so it must be converted to a hook first, before PatchSliceOfMaps can be replaced by a decode hook. --- lib/decode/decode.go | 93 +++++++++++++++++ lib/decode/decode_test.go | 207 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 300 insertions(+) create mode 100644 lib/decode/decode.go create mode 100644 lib/decode/decode_test.go diff --git a/lib/decode/decode.go b/lib/decode/decode.go new file mode 100644 index 0000000000..f6b81911dc --- /dev/null +++ b/lib/decode/decode.go @@ -0,0 +1,93 @@ +/* +Package decode provides tools for customizing the decoding of configuration, +into structures using mapstructure. +*/ +package decode + +import ( + "reflect" + "strings" +) + +// HookTranslateKeys is a mapstructure decode hook which translates keys in a +// map to their canonical value. +// +// Any struct field with a field tag of `alias` may be loaded from any of the +// values keyed by any of the aliases. A field may have one or more alias. +// Aliases must be lowercase, as keys are compared case-insensitive. +// +// Example alias tag: +// MyField []string `alias:"old_field_name,otherfieldname"` +// +// This hook should ONLY be used to maintain backwards compatibility with +// deprecated keys. For new structures use mapstructure struct tags to set the +// desired serialization key. +// +// IMPORTANT: This function assumes that mapstructure is being used with the +// default struct field tag of `mapstructure`. If mapstructure.DecoderConfig.TagName +// is set to a different value this function will need to be parameterized with +// that value to correctly find the canonical data key. +func HookTranslateKeys(_, to reflect.Type, data interface{}) (interface{}, error) { + // Return immediately if target is not a struct, as only structs can have + // field tags. If the target is a pointer to a struct, mapstructure will call + // the hook again with the struct. + if to.Kind() != reflect.Struct { + return data, nil + } + + // Avoid doing any work if data is not a map + source, ok := data.(map[string]interface{}) + if !ok { + return data, nil + } + + rules := translationsForType(to) + for k, v := range source { + lowerK := strings.ToLower(k) + canonKey, ok := rules[lowerK] + if !ok { + continue + } + delete(source, k) + + // if there is a value for the canonical key then keep it + if _, ok := source[canonKey]; ok { + continue + } + source[canonKey] = v + } + return source, nil +} + +// TODO: could be cached if it is too slow +func translationsForType(to reflect.Type) map[string]string { + translations := map[string]string{} + for i := 0; i < to.NumField(); i++ { + field := to.Field(i) + tag, ok := field.Tag.Lookup("alias") + if !ok { + continue + } + + canonKey := strings.ToLower(canonicalFieldKey(field)) + for _, alias := range strings.Split(tag, ",") { + translations[strings.ToLower(alias)] = canonKey + } + } + return translations +} + +func canonicalFieldKey(field reflect.StructField) string { + tag, ok := field.Tag.Lookup("mapstructure") + if !ok { + return field.Name + } + parts := strings.SplitN(tag, ",", 2) + switch { + case len(parts) < 1: + return field.Name + case parts[0] == "": + return field.Name + } + return parts[0] +} diff --git a/lib/decode/decode_test.go b/lib/decode/decode_test.go new file mode 100644 index 0000000000..e701445471 --- /dev/null +++ b/lib/decode/decode_test.go @@ -0,0 +1,207 @@ +package decode + +import ( + "reflect" + "testing" + + "github.com/mitchellh/mapstructure" + "github.com/stretchr/testify/require" +) + +func TestHookTranslateKeys(t *testing.T) { + var testcases = []struct { + name string + data interface{} + expected interface{} + }{ + { + name: "target of type struct, with struct receiver", + data: map[string]interface{}{ + "S": map[string]interface{}{ + "None": "no translation", + "OldOne": "value1", + "oldtwo": "value2", + }, + }, + expected: Config{ + S: TypeStruct{ + One: "value1", + Two: "value2", + None: "no translation", + }, + }, + }, + { + name: "target of type ptr, with struct receiver", + data: map[string]interface{}{ + "PS": map[string]interface{}{ + "None": "no translation", + "OldOne": "value1", + "oldtwo": "value2", + }, + }, + expected: Config{ + PS: &TypeStruct{ + One: "value1", + Two: "value2", + None: "no translation", + }, + }, + }, + { + name: "target of type ptr, with ptr receiver", + data: map[string]interface{}{ + "PTR": map[string]interface{}{ + "None": "no translation", + "old_THREE": "value3", + "oldfour": "value4", + }, + }, + expected: Config{ + PTR: &TypePtrToStruct{ + Three: "value3", + Four: "value4", + None: "no translation", + }, + }, + }, + { + name: "target of type ptr, with struct receiver", + data: map[string]interface{}{ + "PTRS": map[string]interface{}{ + "None": "no translation", + "old_THREE": "value3", + "old_four": "value4", + }, + }, + expected: Config{ + PTRS: TypePtrToStruct{ + Three: "value3", + Four: "value4", + None: "no translation", + }, + }, + }, + { + name: "target of type map", + data: map[string]interface{}{ + "Blob": map[string]interface{}{ + "one": 1, + "two": 2, + }, + }, + expected: Config{ + Blob: map[string]interface{}{ + "one": 1, + "two": 2, + }, + }, + }, + { + name: "value already exists for canonical key", + data: map[string]interface{}{ + "PS": map[string]interface{}{ + "OldOne": "value1", + "One": "original1", + "oldTWO": "value2", + "two": "original2", + }, + }, + expected: Config{ + PS: &TypeStruct{ + One: "original1", + Two: "original2", + }, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + cfg := Config{} + md := new(mapstructure.Metadata) + decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: HookTranslateKeys, + Metadata: md, + Result: &cfg, + }) + require.NoError(t, err) + + require.NoError(t, decoder.Decode(tc.data)) + require.Equal(t, cfg, tc.expected, "decode metadata: %#v", md) + }) + } +} + +type Config struct { + S TypeStruct + PS *TypeStruct + PTR *TypePtrToStruct + PTRS TypePtrToStruct + Blob map[string]interface{} +} + +type TypeStruct struct { + One string `alias:"oldone"` + Two string `alias:"oldtwo"` + None string +} + +type TypePtrToStruct struct { + Three string `alias:"old_three"` + Four string `alias:"old_four,oldfour"` + None string +} + +func TestHookTranslateKeys_TargetStructHasPointerReceiver(t *testing.T) { + target := &TypePtrToStruct{} + md := new(mapstructure.Metadata) + decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: HookTranslateKeys, + Metadata: md, + Result: target, + }) + require.NoError(t, err) + + data := map[string]interface{}{ + "None": "no translation", + "Old_Three": "value3", + "OldFour": "value4", + } + expected := &TypePtrToStruct{ + None: "no translation", + Three: "value3", + Four: "value4", + } + require.NoError(t, decoder.Decode(data)) + require.Equal(t, expected, target, "decode metadata: %#v", md) +} + +type translateExample struct { + FieldDefaultCanonical string `alias:"first"` + FieldWithMapstructureTag string `alias:"second" mapstructure:"field_with_mapstruct_tag"` + FieldWithMapstructureTagOmit string `mapstructure:"field_with_mapstruct_omit,omitempty" alias:"third"` + FieldWithEmptyTag string `mapstructure:"" alias:"forth"` +} + +func TestTranslationsForType(t *testing.T) { + to := reflect.TypeOf(translateExample{}) + actual := translationsForType(to) + expected := map[string]string{ + "first": "fielddefaultcanonical", + "second": "field_with_mapstruct_tag", + "third": "field_with_mapstruct_omit", + "forth": "fieldwithemptytag", + } + require.Equal(t, expected, actual) +} + +type nested struct { + O map[string]interface{} + Slice []Item + Item Item +} + +type Item struct { + Name string +}