Skip to content

Commit

Permalink
fix(fill): Map keys are case-insensitive, handle maps recursively
Browse files Browse the repository at this point in the history
  • Loading branch information
dustmop committed Oct 15, 2020
1 parent facd640 commit ab27f1b
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 17 deletions.
69 changes: 57 additions & 12 deletions base/fill/path_value.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func SetPathValue(path string, val interface{}, output interface{}) error {
target, field, err := findTargetAtPath(steps, target)
if err != nil {
if err == ErrNotFound {
return fmt.Errorf("at \"%s\": path not found", path)
return fmt.Errorf("at %q: %w", path, ErrNotFound)
}
collector.Add(err)
return collector.AsSingleError()
Expand Down Expand Up @@ -65,22 +65,22 @@ func GetPathValue(path string, input interface{}) (interface{}, error) {
// Find target place to assign to, field is a non-empty string only if target is a map.
target, field, err := findTargetAtPath(steps, target)
if err != nil {
if err == ErrNotFound {
return nil, fmt.Errorf("at \"%s\": path not found", path)
}
return nil, fmt.Errorf("at \"%s\": %s", path, err)
return nil, fmt.Errorf("at %q: %w", path, err)
}
// If all steps completed, resolve the target to a real value.
if field == "" {
return target.Interface(), nil
}

lookup := target.MapIndex(reflect.ValueOf(field))
if lookup.IsValid() {
// Otherwise, one step is left, look it up using case-insensitive comparison.
if lookup := mapLookupCaseInsensitive(target, field); lookup != nil {
return lookup.Interface(), nil
}
return nil, fmt.Errorf("at \"%s\": invalid path", path)
return nil, fmt.Errorf("at %q: %w", path, ErrNotFound)
}

// Recursively look up the first step in place, until there are no steps left. If place is ever
// a map with one step left, return that map and final step - this is done in order to enable
// assignment to that map.
func findTargetAtPath(steps []string, place reflect.Value) (reflect.Value, string, error) {
if len(steps) == 0 {
return place, "", nil
Expand All @@ -94,6 +94,16 @@ func findTargetAtPath(steps []string, place reflect.Value) (reflect.Value, strin
return place, "", ErrNotFound
}
return findTargetAtPath(rest, field)
} else if place.Kind() == reflect.Interface {
var inner reflect.Value
if place.IsNil() {
alloc := reflect.New(place.Type().Elem())
place.Set(alloc)
inner = alloc.Elem()
} else {
inner = place.Elem()
}
return findTargetAtPath(steps, inner)
} else if place.Kind() == reflect.Ptr {
var inner reflect.Value
if place.IsNil() {
Expand All @@ -108,9 +118,14 @@ func findTargetAtPath(steps []string, place reflect.Value) (reflect.Value, strin
if place.IsNil() {
place.Set(reflect.MakeMap(place.Type()))
}
// TODO: Handle case where `rest` has more steps and `val` is a struct: more
// recursive is needed.
return place, s, nil
if len(steps) == 1 {
return place, s, nil
}
found := mapLookupCaseInsensitive(place, s)
if found == nil {
return place, "", ErrNotFound
}
return findTargetAtPath(rest, *found)
} else if place.Kind() == reflect.Slice {
num, err := coerceToInt(s)
if err != nil {
Expand All @@ -126,6 +141,36 @@ func findTargetAtPath(steps []string, place reflect.Value) (reflect.Value, strin
}
}

// Look up value in the map, using case-insensitive string comparison. Return nil if not found
func mapLookupCaseInsensitive(mapValue reflect.Value, k string) *reflect.Value {
// Try looking up the value without changing the string case
key := reflect.ValueOf(k)
result := mapValue.MapIndex(key)
if result.IsValid() {
return &result
}
// Try lower-casing the key and looking that up
klower := strings.ToLower(k)
key = reflect.ValueOf(klower)
result = mapValue.MapIndex(key)
if result.IsValid() {
return &result
}
// Iterate over the map keys, compare each, using case-insensitive matching
mapKeys := mapValue.MapKeys()
for _, mk := range mapKeys {
mstr := mk.String()
if strings.ToLower(mstr) == klower {
result = mapValue.MapIndex(mk)
if result.IsValid() {
return &result
}
}
}
// Not found, return nil
return nil
}

func coerceToTargetType(val interface{}, place reflect.Value) (interface{}, error) {
switch place.Kind() {
case reflect.Bool:
Expand Down
91 changes: 86 additions & 5 deletions base/fill/path_value_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ func TestFillPathValue(t *testing.T) {
}

c = Collection{}
err = SetPathValue("not_found", "missing", &c)
expect = `at "not_found": path not found`
err = SetPathValue("non_existent", "missing", &c)
expect = `at "non_existent": not found`
if err == nil {
t.Fatalf("expected: error \"%s\", got no error", expect)
}
Expand Down Expand Up @@ -208,6 +208,10 @@ func TestGetPathValue(t *testing.T) {
"extra": "misc",
},
List: []string{"cat", "dog", "eel"},
Sub: SubElement {
Num: 7,
Text: "sandwich",
},
}

val, err := GetPathValue("name", &c)
Expand All @@ -226,8 +230,24 @@ func TestGetPathValue(t *testing.T) {
t.Errorf("expected: val should be \"misc\"")
}

val, err = GetPathValue("not_found", &c)
expect := `at "not_found": path not found`
val, err = GetPathValue("sub.num", &c)
if err != nil {
panic(err)
}
if val != 7 {
t.Errorf("expected: val should be 7, got %v", val)
}

val, err = GetPathValue("sub.text", &c)
if err != nil {
panic(err)
}
if val != "sandwich" {
t.Errorf("expected: val should be \"sandwich\", got %v", val)
}

val, err = GetPathValue("non_existent", &c)
expect := `at "non_existent": not found`
if err == nil {
t.Fatalf("expected: error \"%s\", got no error", expect)
}
Expand All @@ -236,7 +256,7 @@ func TestGetPathValue(t *testing.T) {
}

val, err = GetPathValue("dict.missing_key", &c)
expect = `at "dict.missing_key": invalid path`
expect = `at "dict.missing_key": not found`
if err == nil {
t.Fatalf("expected: error \"%s\", got no error", expect)
}
Expand All @@ -262,3 +282,64 @@ func TestGetPathValue(t *testing.T) {
}

}

func TestDictKeysCaseInsenstive(t *testing.T) {
obj := map[string]interface{}{
"Parent": map[string]interface{}{
"Child": "ok",
},
"First": map[string]interface{}{
"Second": map[string]interface{}{
"Third": "alright",
},
},
}

val, err := GetPathValue("parent.child", obj)
if err != nil {
panic(err)
}
if val != "ok" {
t.Errorf("expected: val should be \"ok\"")
}

val, err = GetPathValue("parent.Child", obj)
if err != nil {
panic(err)
}
if val != "ok" {
t.Errorf("expected: val should be \"ok\"")
}

val, err = GetPathValue("parent.CHILD", obj)
if err != nil {
panic(err)
}
if val != "ok" {
t.Errorf("expected: val should be \"ok\"")
}

val, err = GetPathValue("Parent.Child", obj)
if err != nil {
panic(err)
}
if val != "ok" {
t.Errorf("expected: val should be \"ok\"")
}

val, err = GetPathValue("first.second.third", obj)
if err != nil {
panic(err)
}
if val != "alright" {
t.Errorf("expected: val should be \"alright\"")
}

val, err = GetPathValue("FIRST.SECOND.THIRD", obj)
if err != nil {
panic(err)
}
if val != "alright" {
t.Errorf("expected: val should be \"alright\"")
}
}
1 change: 1 addition & 0 deletions base/fill/struct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ type SubElement struct {
Num int
Things *map[string]string
Any interface{}
Text string
}

func (c *Collection) SetArbitrary(key string, val interface{}) error {
Expand Down

0 comments on commit ab27f1b

Please sign in to comment.