From e267c41b1d149b5151fb637d65f53b6e833448fd Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Thu, 20 Jul 2023 16:31:20 +0200 Subject: [PATCH] JSON, TOML, YAML: support for nested config data (#112) * internal: add TraverseMap * gofumpt * JSONParseConfig * internal: TraverseMap: map[any]any * fftoml: don't break the API * internal: TraverseMap: test for map[any]any * fftest, ffyaml: nested node tests * Can't remove ParseError types * Can't remove StringConversionError type --- .../objectctl/pkg/objectapi/client.go | 6 +- fftest/tempfile.go | 2 +- fftest/vars.go | 14 ++ fftoml/fftoml.go | 76 ++-------- fftoml/fftoml_test.go | 75 +++++----- ffyaml/ffyaml.go | 83 ++++------- ffyaml/ffyaml_test.go | 6 + ffyaml/testdata/nested.yaml | 13 ++ internal/doc.go | 2 + internal/traverse_map.go | 63 ++++++++ internal/traverse_map_test.go | 135 ++++++++++++++++++ json_parser.go | 86 ++++------- parse.go | 12 ++ 13 files changed, 341 insertions(+), 232 deletions(-) create mode 100644 ffyaml/testdata/nested.yaml create mode 100644 internal/doc.go create mode 100644 internal/traverse_map.go create mode 100644 internal/traverse_map_test.go diff --git a/ffcli/examples/objectctl/pkg/objectapi/client.go b/ffcli/examples/objectctl/pkg/objectapi/client.go index 386eb79..6abd35b 100644 --- a/ffcli/examples/objectctl/pkg/objectapi/client.go +++ b/ffcli/examples/objectctl/pkg/objectapi/client.go @@ -102,17 +102,17 @@ func (s *mockServer) list(token string) ([]Object, error) { } var defaultObjects = map[string]Object{ - "apple": Object{ + "apple": { Key: "apple", Value: "The fruit of any of certain other species of tree of the same genus.", Access: mustParseTime(time.RFC3339, "2019-03-15T15:01:00Z"), }, - "beach": Object{ + "beach": { Key: "beach", Value: "The shore of a body of water, especially when sandy or pebbly.", Access: mustParseTime(time.RFC3339, "2019-04-20T12:21:30Z"), }, - "carillon": Object{ + "carillon": { Key: "carillon", Value: "A stationary set of chromatically tuned bells in a tower.", Access: mustParseTime(time.RFC3339, "2019-07-04T23:59:59Z"), diff --git a/fftest/tempfile.go b/fftest/tempfile.go index cfb5c0f..dd6f7f4 100644 --- a/fftest/tempfile.go +++ b/fftest/tempfile.go @@ -16,7 +16,7 @@ func TempFile(t *testing.T, content string) string { filename := filepath.Join(t.TempDir(), strconv.Itoa(rand.Int())) - if err := os.WriteFile(filename, []byte(content), 0600); err != nil { + if err := os.WriteFile(filename, []byte(content), 0o0600); err != nil { t.Fatal(err) } diff --git a/fftest/vars.go b/fftest/vars.go index d7e3d39..fe2e89a 100644 --- a/fftest/vars.go +++ b/fftest/vars.go @@ -3,6 +3,7 @@ package fftest import ( "errors" "flag" + "fmt" "reflect" "strings" "testing" @@ -44,6 +45,19 @@ func NonzeroDefaultVars(fs *flag.FlagSet) *Vars { return &v } +// NestedDefaultVars is similar to DefaultVars, but uses nested flag names. +func NestedDefaultVars(delimiter string) func(fs *flag.FlagSet) *Vars { + return func(fs *flag.FlagSet) *Vars { + var v Vars + fs.StringVar(&v.S, fmt.Sprintf("foo%ss", delimiter), "", "string") + fs.IntVar(&v.I, fmt.Sprintf("bar%[1]snested%[1]si", delimiter), 0, "int") + fs.Float64Var(&v.F, fmt.Sprintf("bar%[1]snested%[1]sf", delimiter), 0., "float64") + fs.BoolVar(&v.B, fmt.Sprintf("foo%sb", delimiter), false, "bool") + fs.Var(&v.X, fmt.Sprintf("baz%[1]snested%[1]sx", delimiter), "collection of strings (repeatable)") + return &v + } +} + // Vars are a common set of variables used for testing. type Vars struct { S string diff --git a/fftoml/fftoml.go b/fftoml/fftoml.go index 03e628e..9425e2d 100644 --- a/fftoml/fftoml.go +++ b/fftoml/fftoml.go @@ -4,10 +4,9 @@ package fftoml import ( "fmt" "io" - "strconv" "github.com/pelletier/go-toml" - "github.com/peterbourgon/ff/v3" + "github.com/peterbourgon/ff/v3/internal" ) // Parser is a parser for TOML file format. Flags and their values are read @@ -36,12 +35,16 @@ func New(opts ...Option) (c ConfigFileParser) { // Parse parses the provided io.Reader as a TOML file and uses the provided set function // to set flag names derived from the tables names and their key/value pairs. func (c ConfigFileParser) Parse(r io.Reader, set func(name, value string) error) error { - tree, err := toml.LoadReader(r) - if err != nil { + var m map[string]any + if err := toml.NewDecoder(r).Decode(&m); err != nil { return ParseError{Inner: err} } - return parseTree(tree, "", c.delimiter, set) + if err := internal.TraverseMap(m, c.delimiter, set); err != nil { + return ParseError{Inner: err} + } + + return nil } // Option is a function which changes the behavior of the TOML config file parser. @@ -65,69 +68,6 @@ func WithTableDelimiter(d string) Option { } } -func parseTree(tree *toml.Tree, parent, delimiter string, set func(name, value string) error) error { - for _, key := range tree.Keys() { - name := key - if parent != "" { - name = parent + delimiter + key - } - switch t := tree.Get(key).(type) { - case *toml.Tree: - if err := parseTree(t, name, delimiter, set); err != nil { - return err - } - case interface{}: - values, err := valsToStrs(t) - if err != nil { - return ParseError{Inner: err} - } - for _, value := range values { - if err = set(name, value); err != nil { - return err - } - } - } - } - return nil -} - -func valsToStrs(val interface{}) ([]string, error) { - if vals, ok := val.([]interface{}); ok { - ss := make([]string, len(vals)) - for i := range vals { - s, err := valToStr(vals[i]) - if err != nil { - return nil, err - } - ss[i] = s - } - return ss, nil - } - s, err := valToStr(val) - if err != nil { - return nil, err - } - return []string{s}, nil - -} - -func valToStr(val interface{}) (string, error) { - switch v := val.(type) { - case string: - return v, nil - case bool: - return strconv.FormatBool(v), nil - case uint64: - return strconv.FormatUint(v, 10), nil - case int64: - return strconv.FormatInt(v, 10), nil - case float64: - return strconv.FormatFloat(v, 'g', -1, 64), nil - default: - return "", ff.StringConversionError{Value: val} - } -} - // ParseError wraps all errors originating from the TOML parser. type ParseError struct { Inner error diff --git a/fftoml/fftoml_test.go b/fftoml/fftoml_test.go index 55de143..187d6aa 100644 --- a/fftoml/fftoml_test.go +++ b/fftoml/fftoml_test.go @@ -2,7 +2,9 @@ package fftoml_test import ( "flag" + "fmt" "reflect" + "strings" "testing" "time" @@ -56,61 +58,48 @@ func TestParser(t *testing.T) { func TestParser_WithTables(t *testing.T) { t.Parallel() - type fields struct { - String string - Float float64 - Strings fftest.StringSlice - } - - expected := fields{ - String: "a string", - Float: 1.23, - Strings: fftest.StringSlice{"one", "two", "three"}, - } - - for _, testcase := range []struct { - name string - opts []fftoml.Option - // expectations - stringKey string - floatKey string - stringsKey string - }{ - { - name: "defaults", - stringKey: "string.key", - floatKey: "float.nested.key", - stringsKey: "strings.nested.key", - }, - { - name: "defaults", - opts: []fftoml.Option{fftoml.WithTableDelimiter("-")}, - stringKey: "string-key", - floatKey: "float-nested-key", - stringsKey: "strings-nested-key", - }, + for _, delim := range []string{ + ".", + "-", } { - t.Run(testcase.name, func(t *testing.T) { + t.Run(fmt.Sprintf("delim=%q", delim), func(t *testing.T) { var ( - found fields - fs = flag.NewFlagSet("fftest", flag.ContinueOnError) + skey = strings.Join([]string{"string", "key"}, delim) + fkey = strings.Join([]string{"float", "nested", "key"}, delim) + xkey = strings.Join([]string{"strings", "nested", "key"}, delim) + + sval string + fval float64 + xval fftest.StringSlice ) - fs.StringVar(&found.String, testcase.stringKey, "", "string") - fs.Float64Var(&found.Float, testcase.floatKey, 0, "float64") - fs.Var(&found.Strings, testcase.stringsKey, "string slice") + fs := flag.NewFlagSet("fftest", flag.ContinueOnError) + { + fs.StringVar(&sval, skey, "xxx", "string") + fs.Float64Var(&fval, fkey, 999, "float64") + fs.Var(&xval, xkey, "strings") + } + + parseConfig := fftoml.New(fftoml.WithTableDelimiter(delim)) if err := ff.Parse(fs, []string{}, ff.WithConfigFile("testdata/table.toml"), - ff.WithConfigFileParser(fftoml.New(testcase.opts...).Parse), + ff.WithConfigFileParser(parseConfig.Parse), ); err != nil { t.Fatal(err) } - if !reflect.DeepEqual(expected, found) { - t.Errorf(`expected %v, to be %v`, found, expected) + if want, have := "a string", sval; want != have { + t.Errorf("string key: want %q, have %q", want, have) + } + + if want, have := 1.23, fval; want != have { + t.Errorf("float nested key: want %v, have %v", want, have) + } + + if want, have := (fftest.StringSlice{"one", "two", "three"}), xval; !reflect.DeepEqual(want, have) { + t.Errorf("strings nested key: want %v, have %v", want, have) } }) } - } diff --git a/ffyaml/ffyaml.go b/ffyaml/ffyaml.go index 8fe88e4..3f86aba 100644 --- a/ffyaml/ffyaml.go +++ b/ffyaml/ffyaml.go @@ -2,77 +2,44 @@ package ffyaml import ( + "errors" "fmt" "io" - "strconv" - "github.com/peterbourgon/ff/v3" + "github.com/peterbourgon/ff/v3/internal" "gopkg.in/yaml.v2" ) -// Parser is a parser for YAML file format. Flags and their values are read -// from the key/value pairs defined in the config file. +// Parser is a helper function that uses a default ParseConfig. func Parser(r io.Reader, set func(name, value string) error) error { - var m map[string]interface{} - d := yaml.NewDecoder(r) - if err := d.Decode(&m); err != nil && err != io.EOF { - return ParseError{err} - } - for key, val := range m { - values, err := valsToStrs(val) - if err != nil { - return ParseError{err} - } - for _, value := range values { - if err := set(key, value); err != nil { - return err - } - } - } - return nil + return (&ParseConfig{}).Parse(r, set) } -func valsToStrs(val interface{}) ([]string, error) { - if vals, ok := val.([]interface{}); ok { - ss := make([]string, len(vals)) - for i := range vals { - s, err := valToStr(vals[i]) - if err != nil { - return nil, err - } - ss[i] = s - } - return ss, nil - } - s, err := valToStr(val) - if err != nil { - return nil, err +// ParseConfig collects parameters for the YAML config file parser. +type ParseConfig struct { + // Delimiter is used when concatenating nested node keys into a flag name. + // The default delimiter is ".". + Delimiter string +} + +// Parse a YAML document from the provided io.Reader, using the provided set +// function to set flag values. Flag names are derived from the node names and +// their key/value pairs. +func (pc *ParseConfig) Parse(r io.Reader, set func(name, value string) error) error { + if pc.Delimiter == "" { + pc.Delimiter = "." } - return []string{s}, nil -} + var m map[string]interface{} + if err := yaml.NewDecoder(r).Decode(&m); err != nil && !errors.Is(err, io.EOF) { + return ParseError{Inner: err} + } -func valToStr(val interface{}) (string, error) { - switch v := val.(type) { - case byte: - return string([]byte{v}), nil - case string: - return v, nil - case bool: - return strconv.FormatBool(v), nil - case uint64: - return strconv.FormatUint(v, 10), nil - case int: - return strconv.Itoa(v), nil - case int64: - return strconv.FormatInt(v, 10), nil - case float64: - return strconv.FormatFloat(v, 'g', -1, 64), nil - case nil: - return "", nil - default: - return "", ff.StringConversionError{Value: val} + if err := internal.TraverseMap(m, pc.Delimiter, set); err != nil { + return ParseError{Inner: err} } + + return nil } // ParseError wraps all errors originating from the YAML parser. diff --git a/ffyaml/ffyaml_test.go b/ffyaml/ffyaml_test.go index 7667c09..16aed9b 100644 --- a/ffyaml/ffyaml_test.go +++ b/ffyaml/ffyaml_test.go @@ -80,6 +80,12 @@ func TestParser(t *testing.T) { miss: false, want: fftest.Vars{WantParseErrorIs: os.ErrNotExist}, }, + { + name: "nested nodes", + file: "testdata/nested.yaml", + vars: fftest.NestedDefaultVars("."), + want: fftest.Vars{S: "a string", B: true, I: 123, F: 1.23, X: []string{"one", "two", "three"}}, + }, } { t.Run(testcase.name, func(t *testing.T) { if testcase.vars == nil { diff --git a/ffyaml/testdata/nested.yaml b/ffyaml/testdata/nested.yaml new file mode 100644 index 0000000..0b18c00 --- /dev/null +++ b/ffyaml/testdata/nested.yaml @@ -0,0 +1,13 @@ +foo: + s: a string + b: true +bar: + nested: + i: 123 + f: 1.23 +baz: + nested: + x: + - one + - two + - three diff --git a/internal/doc.go b/internal/doc.go new file mode 100644 index 0000000..e144ef5 --- /dev/null +++ b/internal/doc.go @@ -0,0 +1,2 @@ +// Package internal provides private helpers used by various module packages. +package internal diff --git a/internal/traverse_map.go b/internal/traverse_map.go new file mode 100644 index 0000000..410a60e --- /dev/null +++ b/internal/traverse_map.go @@ -0,0 +1,63 @@ +package internal + +import ( + "encoding/json" + "fmt" + "strconv" +) + +// TraverseMap recursively walks the given map, calling set for each value. If the +// value is a slice, set is called for each element of the slice. The keys of +// nested maps are joined with the given delimiter. +func TraverseMap(m map[string]any, delimiter string, set func(name, value string) error) error { + return traverseMap("", m, delimiter, set) +} + +func traverseMap(key string, val any, delimiter string, set func(name, value string) error) error { + switch v := val.(type) { + case string: + return set(key, v) + case json.Number: + return set(key, v.String()) + case uint64: + return set(key, strconv.FormatUint(v, 10)) + case int: + return set(key, strconv.Itoa(v)) + case int64: + return set(key, strconv.FormatInt(v, 10)) + case float64: + return set(key, strconv.FormatFloat(v, 'g', -1, 64)) + case bool: + return set(key, strconv.FormatBool(v)) + case nil: + return set(key, "") + case []any: + for _, v := range v { + if err := traverseMap(key, v, delimiter, set); err != nil { + return err + } + } + case map[string]any: + for k, v := range v { + if key != "" { + k = key + delimiter + k + } + if err := traverseMap(k, v, delimiter, set); err != nil { + return err + } + } + case map[any]any: + for k, v := range v { + ks := fmt.Sprint(k) + if key != "" { + ks = key + delimiter + ks + } + if err := traverseMap(ks, v, delimiter, set); err != nil { + return err + } + } + default: + return fmt.Errorf("couldn't convert %q (type %T) to string", val, val) + } + return nil +} diff --git a/internal/traverse_map_test.go b/internal/traverse_map_test.go new file mode 100644 index 0000000..08de9b3 --- /dev/null +++ b/internal/traverse_map_test.go @@ -0,0 +1,135 @@ +package internal_test + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/peterbourgon/ff/v3/internal" +) + +func TestTraverseMap(t *testing.T) { + t.Parallel() + + tests := []struct { + Name string + M map[string]any + Delim string + Want map[string]struct{} + }{ + { + Name: "single values", + M: map[string]any{ + "s": "foo", + "i": 123, + "i64": int64(123), + "u": uint64(123), + "f": 1.23, + "jn": json.Number("123"), + "b": true, + "nil": nil, + }, + Delim: ".", + Want: map[string]struct{}{ + "s=foo": {}, + "i=123": {}, + "i64=123": {}, + "u=123": {}, + "f=1.23": {}, + "jn=123": {}, + "b=true": {}, + "nil=": {}, + }, + }, + { + Name: "slices", + M: map[string]any{ + "is": []any{1, 2, 3}, + "ss": []any{"a", "b", "c"}, + "bs": []any{true, false}, + "as": []any{"a", 1, true}, + }, + Want: map[string]struct{}{ + "is=1": {}, + "is=2": {}, + "is=3": {}, + "ss=a": {}, + "ss=b": {}, + "ss=c": {}, + "bs=true": {}, + "bs=false": {}, + "as=a": {}, + "as=1": {}, + "as=true": {}, + }, + }, + { + Name: "nested maps", + M: map[string]any{ + "m": map[string]any{ + "s": "foo", + "m2": map[string]any{ + "i": 123, + }, + }, + }, + Delim: ".", + Want: map[string]struct{}{ + "m.s=foo": {}, + "m.m2.i=123": {}, + }, + }, + { + Name: "nested maps with '-' delimiter", + M: map[string]any{ + "m": map[string]any{ + "s": "foo", + "m2": map[string]any{ + "i": 123, + }, + }, + }, + Delim: "-", + Want: map[string]struct{}{ + "m-s=foo": {}, + "m-m2-i=123": {}, + }, + }, + { + Name: "nested map[any]any", + M: map[string]any{ + "m": map[any]any{ + "m2": map[string]any{ + "i": 999, + }, + }, + }, + Delim: ".", + Want: map[string]struct{}{ + "m.m2.i=999": {}, + }, + }, + } + + for _, test := range tests { + t.Run(test.Name, func(t *testing.T) { + observe := func(name, value string) error { + key := name + "=" + value + if _, ok := test.Want[key]; !ok { + t.Errorf("set(%s, %s): unexpected call to set", name, value) + } + delete(test.Want, key) + return nil + } + + if err := internal.TraverseMap(test.M, test.Delim, observe); err != nil { + t.Fatal(err) + } + + for key := range test.Want { + name, value, _ := strings.Cut(key, "=") + t.Errorf("set(%s, %s): expected but did not occur", name, value) + } + }) + } +} diff --git a/json_parser.go b/json_parser.go index 41da564..578759f 100644 --- a/json_parser.go +++ b/json_parser.go @@ -4,64 +4,43 @@ import ( "encoding/json" "fmt" "io" - "strconv" + + "github.com/peterbourgon/ff/v3/internal" ) -// JSONParser is a parser for config files in JSON format. Input should be -// an object. The object's keys are treated as flag names, and the object's -// values as flag values. If the value is an array, the flag will be set -// multiple times. +// JSONParser is a helper function that uses a default JSONParseConfig. func JSONParser(r io.Reader, set func(name, value string) error) error { - var m map[string]interface{} + return (&JSONParseConfig{}).Parse(r, set) +} + +// JSONParseConfig collects parameters for the JSON config file parser. +type JSONParseConfig struct { + // Delimiter is used when concatenating nested node keys into a flag name. + // The default delimiter is ".". + Delimiter string +} + +// Parse a JSON document from the provided io.Reader, using the provided set +// function to set flag values. Flag names are derived from the node names and +// their key/value pairs. +func (pc *JSONParseConfig) Parse(r io.Reader, set func(name, value string) error) error { + if pc.Delimiter == "" { + pc.Delimiter = "." + } + d := json.NewDecoder(r) - d.UseNumber() // must set UseNumber for stringifyValue to work + d.UseNumber() // required for stringifying values + + var m map[string]interface{} if err := d.Decode(&m); err != nil { return JSONParseError{Inner: err} } - for key, val := range m { - values, err := stringifySlice(val) - if err != nil { - return JSONParseError{Inner: err} - } - for _, value := range values { - if err := set(key, value); err != nil { - return err - } - } - } - return nil -} -func stringifySlice(val interface{}) ([]string, error) { - if vals, ok := val.([]interface{}); ok { - ss := make([]string, len(vals)) - for i := range vals { - s, err := stringifyValue(vals[i]) - if err != nil { - return nil, err - } - ss[i] = s - } - return ss, nil - } - s, err := stringifyValue(val) - if err != nil { - return nil, err + if err := internal.TraverseMap(m, pc.Delimiter, set); err != nil { + return JSONParseError{Inner: err} } - return []string{s}, nil -} -func stringifyValue(val interface{}) (string, error) { - switch v := val.(type) { - case string: - return v, nil - case json.Number: - return v.String(), nil - case bool: - return strconv.FormatBool(v), nil - default: - return "", StringConversionError{Value: val} - } + return nil } // JSONParseError wraps all errors originating from the JSONParser. @@ -79,14 +58,3 @@ func (e JSONParseError) Error() string { func (e JSONParseError) Unwrap() error { return e.Inner } - -// StringConversionError is returned when a value in a config file -// can't be converted to a string, to be provided to a flag. -type StringConversionError struct { - Value interface{} -} - -// Error implements the error interface. -func (e StringConversionError) Error() string { - return fmt.Sprintf("couldn't convert %q (type %T) to string", e.Value, e.Value) -} diff --git a/parse.go b/parse.go index a73c74a..4735951 100644 --- a/parse.go +++ b/parse.go @@ -307,3 +307,15 @@ func maybeSplit(value, split string) []string { } return strings.Split(value, split) } + +// StringConversionError was returned by config file parsers in certain cases. +// +// DEPRECATED: this error is no longer returned by anything. +type StringConversionError struct { + Value interface{} +} + +// Error implements the error interface. +func (e StringConversionError) Error() string { + return fmt.Sprintf("couldn't convert %q (type %T) to string", e.Value, e.Value) +}