From 2eb4a375335a6b798e5aee88f3ffffaf2e52733b Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Thu, 4 Apr 2019 12:52:43 -0400 Subject: [PATCH] mcfg: make Parse and Populate return a Context, to allow for sub-commands --- mcfg/cli.go | 12 ++++++------ mcfg/cli_test.go | 4 ++-- mcfg/env.go | 6 +++--- mcfg/mcfg.go | 17 +++++++++++------ mcfg/mcfg_test.go | 6 +++--- mcfg/source.go | 22 ++++++++++++++-------- mcfg/source_test.go | 6 +++--- 7 files changed, 42 insertions(+), 31 deletions(-) diff --git a/mcfg/cli.go b/mcfg/cli.go index 61d94f3..ae104ea 100644 --- a/mcfg/cli.go +++ b/mcfg/cli.go @@ -75,7 +75,7 @@ const ( ) // Parse implements the method for the Source interface -func (cli *SourceCLI) Parse(ctx context.Context, params []Param) ([]ParamValue, error) { +func (cli *SourceCLI) Parse(ctx context.Context, params []Param) (context.Context, []ParamValue, error) { args := cli.Args if cli.Args == nil { args = os.Args[1:] @@ -83,7 +83,7 @@ func (cli *SourceCLI) Parse(ctx context.Context, params []Param) ([]ParamValue, pM, err := cli.cliParams(params) if err != nil { - return nil, err + return nil, nil, err } pvs := make([]ParamValue, 0, len(args)) var ( @@ -119,10 +119,10 @@ func (cli *SourceCLI) Parse(ctx context.Context, params []Param) ([]ParamValue, } if !pOk { if ok := populateCLITail(ctx, args[i:]); ok { - return pvs, nil + return ctx, pvs, nil } ctx := mctx.Annotate(context.Background(), "param", arg) - return nil, merr.New("unexpected config parameter", ctx) + return nil, nil, merr.New("unexpected config parameter", ctx) } } @@ -153,10 +153,10 @@ func (cli *SourceCLI) Parse(ctx context.Context, params []Param) ([]ParamValue, } if pOk && !pvStrValOk { ctx := mctx.Annotate(p.Context, "param", key) - return nil, merr.New("param expected a value", ctx) + return nil, nil, merr.New("param expected a value", ctx) } - return pvs, nil + return ctx, pvs, nil } func (cli *SourceCLI) cliParams(params []Param) (map[string]Param, error) { diff --git a/mcfg/cli_test.go b/mcfg/cli_test.go index 3b63982..e194743 100644 --- a/mcfg/cli_test.go +++ b/mcfg/cli_test.go @@ -143,7 +143,7 @@ func TestWithCLITail(t *T) { for _, tc := range cases { ctx, tail := WithCLITail(ctx) - err := Populate(ctx, &SourceCLI{Args: tc.args}) + _, err := Populate(ctx, &SourceCLI{Args: tc.args}) massert.Require(t, massert.Comment(massert.All( massert.Nil(err), massert.Equal(tc.expTail, *tail), @@ -157,7 +157,7 @@ func ExampleWithCLITail() { ctx, tail := WithCLITail(ctx) ctx, bar := WithString(ctx, "bar", "defaultVal", "Description of bar.") - err := Populate(ctx, &SourceCLI{ + _, err := Populate(ctx, &SourceCLI{ Args: []string{"--foo=100", "BADARG", "--bar", "BAR"}, }) diff --git a/mcfg/env.go b/mcfg/env.go index 47f472f..8864b98 100644 --- a/mcfg/env.go +++ b/mcfg/env.go @@ -43,7 +43,7 @@ func (env *SourceEnv) expectedName(path []string, name string) string { } // Parse implements the method for the Source interface -func (env *SourceEnv) Parse(ctx context.Context, params []Param) ([]ParamValue, error) { +func (env *SourceEnv) Parse(ctx context.Context, params []Param) (context.Context, []ParamValue, error) { kvs := env.Env if kvs == nil { kvs = os.Environ() @@ -60,7 +60,7 @@ func (env *SourceEnv) Parse(ctx context.Context, params []Param) ([]ParamValue, split := strings.SplitN(kv, "=", 2) if len(split) != 2 { ctx := mctx.Annotate(context.Background(), "kv", kv) - return nil, merr.New("malformed environment key/value pair", ctx) + return nil, nil, merr.New("malformed environment key/value pair", ctx) } k, v := split[0], split[1] if p, ok := pM[k]; ok { @@ -72,5 +72,5 @@ func (env *SourceEnv) Parse(ctx context.Context, params []Param) ([]ParamValue, } } - return pvs, nil + return ctx, pvs, nil } diff --git a/mcfg/mcfg.go b/mcfg/mcfg.go index b582482..79e63a6 100644 --- a/mcfg/mcfg.go +++ b/mcfg/mcfg.go @@ -77,9 +77,14 @@ func paramHash(path []string, name string) string { // multiple times with the same Context, each time will only affect the values // of the Params which were provided by the respective Source. // +// Populating Params can affect the Context itself, for example in the case of +// sub-commands. For this reason Populate will return a Context instance which +// may have been affected by the Params (or, if not, will be the same one which +// was passed in). +// // Source may be nil to indicate that no configuration is provided. Only default // values will be used, and if any parameters are required this will error. -func Populate(ctx context.Context, src Source) error { +func Populate(ctx context.Context, src Source) (context.Context, error) { params := collectParams(ctx) if src == nil { src = ParamValues(nil) @@ -97,9 +102,9 @@ func Populate(ctx context.Context, src Source) error { pM[hash] = p } - pvs, err := src.Parse(ctx, params) + ctx, pvs, err := src.Parse(ctx, params) if err != nil { - return err + return nil, err } // dedupe the ParamValues based on their hashes, with the last ParamValue @@ -120,7 +125,7 @@ func Populate(ctx context.Context, src Source) error { } else if _, ok := pvM[hash]; !ok { ctx := mctx.Annotate(p.Context, "param", paramFullName(mctx.Path(p.Context), p.Name)) - return merr.New("required parameter is not set", ctx) + return nil, merr.New("required parameter is not set", ctx) } } @@ -129,9 +134,9 @@ func Populate(ctx context.Context, src Source) error { // at this point, all ParamValues in pvM have a corresponding pM Param p := pM[hash] if err := json.Unmarshal(pv.Value, p.Into); err != nil { - return err + return nil, err } } - return nil + return ctx, nil } diff --git a/mcfg/mcfg_test.go b/mcfg/mcfg_test.go index 1c5b8e3..583c756 100644 --- a/mcfg/mcfg_test.go +++ b/mcfg/mcfg_test.go @@ -17,7 +17,7 @@ func TestPopulate(t *T) { ctxChild, c := WithInt(ctxChild, "c", 0, "") ctx = mctx.WithChild(ctx, ctxChild) - err := Populate(ctx, &SourceCLI{ + _, err := Populate(ctx, &SourceCLI{ Args: []string{"--a=1", "--foo-b=2"}, }) assert.NoError(t, err) @@ -34,12 +34,12 @@ func TestPopulate(t *T) { ctxChild, c := WithRequiredInt(ctxChild, "c", "") ctx = mctx.WithChild(ctx, ctxChild) - err := Populate(ctx, &SourceCLI{ + _, err := Populate(ctx, &SourceCLI{ Args: []string{"--a=1", "--foo-b=2"}, }) assert.Error(t, err) - err = Populate(ctx, &SourceCLI{ + _, err = Populate(ctx, &SourceCLI{ Args: []string{"--a=1", "--foo-b=2", "--foo-c=3"}, }) assert.NoError(t, err) diff --git a/mcfg/source.go b/mcfg/source.go index c3f9cc4..29f4266 100644 --- a/mcfg/source.go +++ b/mcfg/source.go @@ -17,6 +17,11 @@ type ParamValue struct { // sorted set of possible Params to parse, and the Context from with the Params // were extracted. // +// It's possible for Parsing to affect the Context itself, for example in the +// case of sub-commands. For this reason Parse can return a Context, which will +// get used for subsequent Parse commands inside, and then returned from, +// Populate. +// // Source should not return ParamValues which were not explicitly set to a value // by the configuration source. // @@ -25,7 +30,7 @@ type ParamValue struct { // ParamValues which do not correspond to any of the passed in Params. These // will be ignored in Populate. type Source interface { - Parse(context.Context, []Param) ([]ParamValue, error) + Parse(context.Context, []Param) (context.Context, []ParamValue, error) } // ParamValues is simply a slice of ParamValue elements, which implements Parse @@ -33,8 +38,8 @@ type Source interface { type ParamValues []ParamValue // Parse implements the method for the Source interface. -func (pvs ParamValues) Parse(context.Context, []Param) ([]ParamValue, error) { - return pvs, nil +func (pvs ParamValues) Parse(ctx context.Context, _ []Param) (context.Context, []ParamValue, error) { + return ctx, pvs, nil } // Sources combines together multiple Source instances into one. It will call @@ -43,14 +48,15 @@ func (pvs ParamValues) Parse(context.Context, []Param) ([]ParamValue, error) { type Sources []Source // Parse implements the method for the Source interface. -func (ss Sources) Parse(ctx context.Context, params []Param) ([]ParamValue, error) { +func (ss Sources) Parse(ctx context.Context, params []Param) (context.Context, []ParamValue, error) { var pvs []ParamValue for _, s := range ss { - innerPVs, err := s.Parse(ctx, params) - if err != nil { - return nil, err + var innerPVs []ParamValue + var err error + if ctx, innerPVs, err = s.Parse(ctx, params); err != nil { + return nil, nil, err } pvs = append(pvs, innerPVs...) } - return pvs, nil + return ctx, pvs, nil } diff --git a/mcfg/source_test.go b/mcfg/source_test.go index 67b1dc6..2dafeca 100644 --- a/mcfg/source_test.go +++ b/mcfg/source_test.go @@ -143,7 +143,7 @@ func (scs srcCommonState) applyCtxAndPV(p srcCommonParams) srcCommonState { // ParamValues func (scs srcCommonState) assert(s Source) error { root := scs.mkRoot() - gotPVs, err := s.Parse(root, collectParams(root)) + _, gotPVs, err := s.Parse(root, collectParams(root)) if err != nil { return err } @@ -159,7 +159,7 @@ func TestSources(t *T) { ctx, b := WithRequiredInt(ctx, "b", "") ctx, c := WithRequiredInt(ctx, "c", "") - err := Populate(ctx, Sources{ + _, err := Populate(ctx, Sources{ &SourceCLI{Args: []string{"--a=1", "--b=666"}}, &SourceEnv{Env: []string{"B=2", "C=3"}}, }) @@ -179,7 +179,7 @@ func TestSourceParamValues(t *T) { foo, c := WithBool(foo, "c", "") ctx = mctx.WithChild(ctx, foo) - err := Populate(ctx, ParamValues{ + _, err := Populate(ctx, ParamValues{ {Name: "a", Value: json.RawMessage(`4`)}, {Path: []string{"foo"}, Name: "b", Value: json.RawMessage(`"bbb"`)}, {Path: []string{"foo"}, Name: "c", Value: json.RawMessage("true")},