From 6d67515500ea32688ea41ff6044954afae370e03 Mon Sep 17 00:00:00 2001 From: Trevor Foster Date: Fri, 27 Mar 2020 23:21:05 -0400 Subject: [PATCH 1/7] add parsing for stringToString flags --- viper.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/viper.go b/viper.go index 7b12b36e9..1d2610ac7 100644 --- a/viper.go +++ b/viper.go @@ -1083,6 +1083,16 @@ func (v *Viper) find(lcaseKey string, flagDefault bool) interface{} { s = strings.TrimSuffix(s, "]") res, _ := readAsCSV(s) return cast.ToIntSlice(res) + case "stringToString": + s := strings.TrimPrefix(flag.ValueString(), "[") + s = strings.TrimSuffix(s, "]") + elements := strings.Split(s, ",") + result := make(map[string]string, len(elements)) + for _, element := range elements { + pair := strings.SplitN(element, "=", 2) + result[pair[0]] = pair[1] + } + return result default: return flag.ValueString() } From 326772ea293b89ac4256a090f01cdfc623984900 Mon Sep 17 00:00:00 2001 From: Trevor Foster Date: Sat, 28 Mar 2020 00:07:50 -0400 Subject: [PATCH 2/7] add logic to return flags default if not val set, add a test --- viper.go | 16 ++++++++++++++++ viper_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/viper.go b/viper.go index 1d2610ac7..5c38cef4f 100644 --- a/viper.go +++ b/viper.go @@ -1086,6 +1086,9 @@ func (v *Viper) find(lcaseKey string, flagDefault bool) interface{} { case "stringToString": s := strings.TrimPrefix(flag.ValueString(), "[") s = strings.TrimSuffix(s, "]") + if s == "" { + return nil + } elements := strings.Split(s, ",") result := make(map[string]string, len(elements)) for _, element := range elements { @@ -1168,6 +1171,19 @@ func (v *Viper) find(lcaseKey string, flagDefault bool) interface{} { s = strings.TrimSuffix(s, "]") res, _ := readAsCSV(s) return cast.ToIntSlice(res) + case "stringToString": + s := strings.TrimPrefix(flag.ValueString(), "[") + s = strings.TrimSuffix(s, "]") + if s == "" { + return nil + } + elements := strings.Split(s, ",") + result := make(map[string]string, len(elements)) + for _, element := range elements { + pair := strings.SplitN(element, "=", 2) + result[pair[0]] = pair[1] + } + return result default: return flag.ValueString() } diff --git a/viper_test.go b/viper_test.go index b8ceccba5..f0de6be25 100644 --- a/viper_test.go +++ b/viper_test.go @@ -970,6 +970,51 @@ func TestBindPFlag(t *testing.T) { assert.Equal(t, "testing_mutate", Get("testvalue")) } +func TestBindPFlagStringToString(t *testing.T) { + tests := []struct { + Expected map[string]string + Value string + }{ + {nil, ""}, + {map[string]string{"yo": "hi"}, "yo=hi"}, + {map[string]string{"yo": "hi", "oh": "hi=there"}, "yo=hi,oh=hi=there"}, + } + + v := New() // create independent Viper object + defaultVal := map[string]string{} + v.SetDefault("stringtostring", defaultVal) + + for _, testValue := range tests { + flagSet := pflag.NewFlagSet("test", pflag.ContinueOnError) + flagSet.StringToString("stringtostring", testValue.Expected, "test") + + for _, changed := range []bool{true, false} { + flagSet.VisitAll(func(f *pflag.Flag) { + f.Value.Set(testValue.Value) + f.Changed = changed + }) + + err := v.BindPFlags(flagSet) + if err != nil { + t.Fatalf("error binding flag set, %v", err) + } + + type TestMap struct { + StringToString map[string]string + } + val := &TestMap{} + if err := v.Unmarshal(val); err != nil { + t.Fatalf("%+#v cannot unmarshal: %s", testValue.Value, err) + } + if changed { + assert.Equal(t, testValue.Expected, val.StringToString) + } else { + assert.Equal(t, defaultVal, val.StringToString) + } + } + } +} + func TestBoundCaseSensitivity(t *testing.T) { assert.Equal(t, "brown", Get("eyes")) From 61c17d0781a546c8d7f3540b559cdaf147b9266a Mon Sep 17 00:00:00 2001 From: Trevor Foster Date: Sat, 28 Mar 2020 00:14:46 -0400 Subject: [PATCH 3/7] extract parsing into single func --- viper.go | 41 +++++++++++++++++------------------------ 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/viper.go b/viper.go index 5c38cef4f..2f82f91bf 100644 --- a/viper.go +++ b/viper.go @@ -1084,18 +1084,7 @@ func (v *Viper) find(lcaseKey string, flagDefault bool) interface{} { res, _ := readAsCSV(s) return cast.ToIntSlice(res) case "stringToString": - s := strings.TrimPrefix(flag.ValueString(), "[") - s = strings.TrimSuffix(s, "]") - if s == "" { - return nil - } - elements := strings.Split(s, ",") - result := make(map[string]string, len(elements)) - for _, element := range elements { - pair := strings.SplitN(element, "=", 2) - result[pair[0]] = pair[1] - } - return result + return parseStringToStringFlagValue(flag.ValueString()) default: return flag.ValueString() } @@ -1172,18 +1161,7 @@ func (v *Viper) find(lcaseKey string, flagDefault bool) interface{} { res, _ := readAsCSV(s) return cast.ToIntSlice(res) case "stringToString": - s := strings.TrimPrefix(flag.ValueString(), "[") - s = strings.TrimSuffix(s, "]") - if s == "" { - return nil - } - elements := strings.Split(s, ",") - result := make(map[string]string, len(elements)) - for _, element := range elements { - pair := strings.SplitN(element, "=", 2) - result[pair[0]] = pair[1] - } - return result + return parseStringToStringFlagValue(flag.ValueString()) default: return flag.ValueString() } @@ -1203,6 +1181,21 @@ func readAsCSV(val string) ([]string, error) { return csvReader.Read() } +func parseStringToStringFlagValue(val string) map[string]string { + s := strings.TrimPrefix(val, "[") + s = strings.TrimSuffix(s, "]") + if s == "" { + return nil + } + elements := strings.Split(s, ",") + result := make(map[string]string, len(elements)) + for _, element := range elements { + pair := strings.SplitN(element, "=", 2) + result[pair[0]] = pair[1] + } + return result +} + // IsSet checks to see if the key has been set in any of the data locations. // IsSet is case-insensitive for a key. func IsSet(key string) bool { return v.IsSet(key) } From 21e376d12e6289ff1d18d646fd27cfbdd28a655b Mon Sep 17 00:00:00 2001 From: Trevor Foster Date: Sat, 28 Mar 2020 22:47:35 -0400 Subject: [PATCH 4/7] add a few more cases --- viper_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/viper_test.go b/viper_test.go index f0de6be25..9682dcb58 100644 --- a/viper_test.go +++ b/viper_test.go @@ -978,6 +978,8 @@ func TestBindPFlagStringToString(t *testing.T) { {nil, ""}, {map[string]string{"yo": "hi"}, "yo=hi"}, {map[string]string{"yo": "hi", "oh": "hi=there"}, "yo=hi,oh=hi=there"}, + {map[string]string{"yo": ""}, "yo="}, + {map[string]string{"yo": "", "oh": "hi=there"}, "yo=,oh=hi=there"}, } v := New() // create independent Viper object From d6288da8b8a7a7de3d3bd880deaa8ab9e6c347be Mon Sep 17 00:00:00 2001 From: Trevor Foster Date: Sat, 28 Mar 2020 23:20:43 -0400 Subject: [PATCH 5/7] return nil if unable to parse instead of panicing --- viper.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/viper.go b/viper.go index 2f82f91bf..dc2511421 100644 --- a/viper.go +++ b/viper.go @@ -1191,6 +1191,9 @@ func parseStringToStringFlagValue(val string) map[string]string { result := make(map[string]string, len(elements)) for _, element := range elements { pair := strings.SplitN(element, "=", 2) + if len(pair) != 2 { + return nil + } result[pair[0]] = pair[1] } return result From 018de44cf77382dc0689bd08144a90dfc9825b7d Mon Sep 17 00:00:00 2001 From: Trevor Foster Date: Thu, 30 Apr 2020 01:28:08 -0400 Subject: [PATCH 6/7] return map[string]interface in order to work with cast.ToStringMap --- viper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/viper.go b/viper.go index dc2511421..352c77d54 100644 --- a/viper.go +++ b/viper.go @@ -1181,14 +1181,14 @@ func readAsCSV(val string) ([]string, error) { return csvReader.Read() } -func parseStringToStringFlagValue(val string) map[string]string { +func parseStringToStringFlagValue(val string) map[string]interface{} { s := strings.TrimPrefix(val, "[") s = strings.TrimSuffix(s, "]") if s == "" { return nil } elements := strings.Split(s, ",") - result := make(map[string]string, len(elements)) + result := make(map[string]interface{}, len(elements)) for _, element := range elements { pair := strings.SplitN(element, "=", 2) if len(pair) != 2 { From c633cc5570cce917f8bf029c5c54da8a219e56fd Mon Sep 17 00:00:00 2001 From: Trevor Foster Date: Tue, 5 May 2020 17:23:41 -0400 Subject: [PATCH 7/7] mostly copy pflags implementation of the conversion to a stringtostring --- viper.go | 32 +++++++++++++++++++------------- viper_test.go | 2 +- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/viper.go b/viper.go index 352c77d54..65641f665 100644 --- a/viper.go +++ b/viper.go @@ -1084,7 +1084,7 @@ func (v *Viper) find(lcaseKey string, flagDefault bool) interface{} { res, _ := readAsCSV(s) return cast.ToIntSlice(res) case "stringToString": - return parseStringToStringFlagValue(flag.ValueString()) + return stringToStringConv(flag.ValueString()) default: return flag.ValueString() } @@ -1161,7 +1161,7 @@ func (v *Viper) find(lcaseKey string, flagDefault bool) interface{} { res, _ := readAsCSV(s) return cast.ToIntSlice(res) case "stringToString": - return parseStringToStringFlagValue(flag.ValueString()) + return stringToStringConv(flag.ValueString()) default: return flag.ValueString() } @@ -1181,22 +1181,28 @@ func readAsCSV(val string) ([]string, error) { return csvReader.Read() } -func parseStringToStringFlagValue(val string) map[string]interface{} { - s := strings.TrimPrefix(val, "[") - s = strings.TrimSuffix(s, "]") - if s == "" { +// mostly copied from pflag's implementation of this operation here https://github.com/spf13/pflag/blob/master/string_to_string.go#L79 +// alterations are: errors are swallowed, map[string]interface{} is returned in order to enable cast.ToStringMap +func stringToStringConv(val string) interface{} { + val = strings.Trim(val, "[]") + // An empty string would cause an empty map + if len(val) == 0 { + return map[string]interface{}{} + } + r := csv.NewReader(strings.NewReader(val)) + ss, err := r.Read() + if err != nil { return nil } - elements := strings.Split(s, ",") - result := make(map[string]interface{}, len(elements)) - for _, element := range elements { - pair := strings.SplitN(element, "=", 2) - if len(pair) != 2 { + out := make(map[string]interface{}, len(ss)) + for _, pair := range ss { + kv := strings.SplitN(pair, "=", 2) + if len(kv) != 2 { return nil } - result[pair[0]] = pair[1] + out[kv[0]] = kv[1] } - return result + return out } // IsSet checks to see if the key has been set in any of the data locations. diff --git a/viper_test.go b/viper_test.go index 9682dcb58..fe942de29 100644 --- a/viper_test.go +++ b/viper_test.go @@ -975,7 +975,7 @@ func TestBindPFlagStringToString(t *testing.T) { Expected map[string]string Value string }{ - {nil, ""}, + {map[string]string{}, ""}, {map[string]string{"yo": "hi"}, "yo=hi"}, {map[string]string{"yo": "hi", "oh": "hi=there"}, "yo=hi,oh=hi=there"}, {map[string]string{"yo": ""}, "yo="},