From 8e2cffd65b60b12fd9f3a8b47492b08a2eb270d0 Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Fri, 25 Jan 2019 17:33:36 -0500 Subject: [PATCH] mcfg: make ParamValue not embed Param, so that a Source which is []ParamValue makes sense, and can replace SourceMap --- mcfg/cli.go | 21 +++++++++--------- mcfg/env.go | 6 +++--- mcfg/mcfg.go | 52 ++++++++++++++++++++++++++++++++++++--------- mcfg/param.go | 21 ++++-------------- mcfg/source.go | 37 ++++++++++++-------------------- mcfg/source_test.go | 12 +++++------ 6 files changed, 80 insertions(+), 69 deletions(-) diff --git a/mcfg/cli.go b/mcfg/cli.go index 4393669..ff0b556 100644 --- a/mcfg/cli.go +++ b/mcfg/cli.go @@ -58,7 +58,7 @@ func (cli SourceCLI) Parse(params []Param) ([]ParamValue, error) { pvs := make([]ParamValue, 0, len(args)) var ( key string - pv ParamValue + p Param pvOk bool pvStrVal string pvStrValOk bool @@ -72,7 +72,7 @@ func (cli SourceCLI) Parse(params []Param) ([]ParamValue, error) { os.Stdout.Sync() os.Exit(1) } else { - for key, pv.Param = range pM { + for key, p = range pM { if arg == key { pvOk = true break @@ -88,8 +88,7 @@ func (cli SourceCLI) Parse(params []Param) ([]ParamValue, error) { break } if !pvOk { - err := merr.New("unexpected config parameter") - return nil, merr.WithValue(err, "param", arg, true) + return nil, merr.New("unexpected config parameter", "param", arg) } } @@ -97,7 +96,7 @@ func (cli SourceCLI) Parse(params []Param) ([]ParamValue, error) { // As a special case for CLI, if a boolean has no value set it means it // is true. - if pv.IsBool && !pvStrValOk { + if p.IsBool && !pvStrValOk { pvStrVal = "true" pvStrValOk = true } else if !pvStrValOk { @@ -107,18 +106,20 @@ func (cli SourceCLI) Parse(params []Param) ([]ParamValue, error) { continue } - pv.Value = pv.Param.fuzzyParse(pvStrVal) + pvs = append(pvs, ParamValue{ + Name: p.Name, + Path: p.Path, + Value: p.fuzzyParse(pvStrVal), + }) - pvs = append(pvs, pv) key = "" - pv = ParamValue{} + p = Param{} pvOk = false pvStrVal = "" pvStrValOk = false } if pvOk && !pvStrValOk { - err := merr.New("param expected a value") - return nil, merr.WithValue(err, "param", key, true) + return nil, merr.New("param expected a value", "param", key) } return pvs, nil } diff --git a/mcfg/env.go b/mcfg/env.go index 026fa49..3eb977b 100644 --- a/mcfg/env.go +++ b/mcfg/env.go @@ -57,13 +57,13 @@ func (env SourceEnv) Parse(params []Param) ([]ParamValue, error) { for _, kv := range kvs { split := strings.SplitN(kv, "=", 2) if len(split) != 2 { - err := merr.New("malformed environment key/value pair") - return nil, merr.WithValue(err, "kv", kv, true) + return nil, merr.New("malformed environment key/value pair", "kv", kv) } k, v := split[0], split[1] if p, ok := pM[k]; ok { pvs = append(pvs, ParamValue{ - Param: p, + Name: p.Name, + Path: p.Path, Value: p.fuzzyParse(v), }) } diff --git a/mcfg/mcfg.go b/mcfg/mcfg.go index 15bcb49..b519fe2 100644 --- a/mcfg/mcfg.go +++ b/mcfg/mcfg.go @@ -3,7 +3,10 @@ package mcfg import ( + "crypto/md5" + "encoding/hex" "encoding/json" + "fmt" "sort" "github.com/mediocregopher/mediocre-go-lib/mctx" @@ -75,9 +78,31 @@ func collectParams(ctx mctx.Context) []Param { return params } +func paramHash(path []string, name string) string { + h := md5.New() + for _, pathEl := range path { + fmt.Fprintf(h, "pathEl:%q\n", pathEl) + } + fmt.Fprintf(h, "name:%q\n", name) + hStr := hex.EncodeToString(h.Sum(nil)) + // we add the displayName to it to make debugging easier + return paramFullName(path, name) + "/" + hStr +} + func populate(params []Param, src Source) error { if src == nil { - src = SourceMap{} + src = ParamValues(nil) + } + + // map Params to their hash, so we can match them to their ParamValues + // later. There should not be any duplicates here. + pM := map[string]Param{} + for _, p := range params { + hash := paramHash(p.Path, p.Name) + if _, ok := pM[hash]; ok { + panic("duplicate Param: " + paramFullName(p.Path, p.Name)) + } + pM[hash] = p } pvs, err := src.Parse(params) @@ -86,24 +111,31 @@ func populate(params []Param, src Source) error { } // dedupe the ParamValues based on their hashes, with the last ParamValue - // taking precedence + // taking precedence. Also filter out those with no corresponding Param. pvM := map[string]ParamValue{} for _, pv := range pvs { - pvM[pv.hash()] = pv + hash := paramHash(pv.Path, pv.Name) + if _, ok := pM[hash]; !ok { + continue + } + pvM[hash] = pv } // check for required params - for _, param := range params { - if !param.Required { + for hash, p := range pM { + if !p.Required { continue - } else if _, ok := pvM[param.hash()]; !ok { - err := merr.New("required parameter is not set") - return merr.WithValue(err, "param", param.fullName(), true) + } else if _, ok := pvM[hash]; !ok { + return merr.New("required parameter is not set", + "param", paramFullName(p.Path, p.Name)) } } - for _, pv := range pvM { - if err := json.Unmarshal(pv.Value, pv.Into); err != nil { + // do the actual populating + for hash, pv := range pvM { + // 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 } } diff --git a/mcfg/param.go b/mcfg/param.go index e699881..abc6101 100644 --- a/mcfg/param.go +++ b/mcfg/param.go @@ -1,8 +1,6 @@ package mcfg import ( - "crypto/md5" - "encoding/hex" "encoding/json" "fmt" "strings" @@ -49,6 +47,10 @@ type Param struct { Path []string } +func paramFullName(path []string, name string) string { + return strings.Join(append(path, name), "-") +} + func (p Param) fuzzyParse(v string) json.RawMessage { if p.IsBool { if v == "" || v == "0" || v == "false" { @@ -63,21 +65,6 @@ func (p Param) fuzzyParse(v string) json.RawMessage { return json.RawMessage(v) } -func (p Param) fullName() string { - return strings.Join(append(p.Path, p.Name), "-") -} - -func (p Param) hash() string { - h := md5.New() - for _, path := range p.Path { - fmt.Fprintf(h, "pathEl:%q\n", path) - } - fmt.Fprintf(h, "name:%q\n", p.Name) - hStr := hex.EncodeToString(h.Sum(nil)) - // we add the displayName to it to make debugging easier - return p.fullName() + "/" + hStr -} - // MustAdd adds the given Param to the mctx.Context. It will panic if a Param of // the same Name already exists in the mctx.Context. func MustAdd(ctx mctx.Context, param Param) { diff --git a/mcfg/source.go b/mcfg/source.go index 7f96842..3bb661d 100644 --- a/mcfg/source.go +++ b/mcfg/source.go @@ -7,7 +7,8 @@ import ( // ParamValue describes a value for a parameter which has been parsed by a // Source. type ParamValue struct { - Param + Name string + Path []string Value json.RawMessage } @@ -18,11 +19,22 @@ type ParamValue struct { // by the configuration source. // // The returned []ParamValue may contain duplicates of the same Param's value. -// in which case the later value takes precedence. +// in which case the later value takes precedence. It may also contain +// ParamValues which do not correspond to any of the passed in Params. These +// will be ignored in Populate. type Source interface { Parse([]Param) ([]ParamValue, error) } +// ParamValues is simply a slice of ParamValue elements, which implements Parse +// by always returning itself as-is. +type ParamValues []ParamValue + +// Parse implements the method for the Source interface. +func (pvs ParamValues) Parse([]Param) ([]ParamValue, error) { + return pvs, nil +} + // Sources combines together multiple Source instances into one. It will call // Parse on each element individually. Values from later Sources take precedence // over previous ones. @@ -40,24 +52,3 @@ func (ss Sources) Parse(params []Param) ([]ParamValue, error) { } return pvs, nil } - -// SourceMap implements the Source interface by mapping parameter names to -// values for them. The names are comprised of the path and name of a Param -// joined by "-" characters, i.e. `strings.Join(append(param.Path, param.Name), -// "-")`. Values will be parsed in the same way that SourceEnv parses its -// variables. -type SourceMap map[string]string - -// Parse implements the method for the Source interface. -func (m SourceMap) Parse(params []Param) ([]ParamValue, error) { - pvs := make([]ParamValue, 0, len(m)) - for _, p := range params { - if v, ok := m[p.fullName()]; ok { - pvs = append(pvs, ParamValue{ - Param: p, - Value: p.fuzzyParse(v), - }) - } - } - return pvs, nil -} diff --git a/mcfg/source_test.go b/mcfg/source_test.go index 4779a0d..3e0a5fe 100644 --- a/mcfg/source_test.go +++ b/mcfg/source_test.go @@ -109,7 +109,7 @@ func (scs srcCommonState) applyCtxAndPV(p srcCommonParams) srcCommonState { ctxP = get(thisCtx).params[p.name] // get it back out to get any added fields if !p.unset { - pv := ParamValue{Param: ctxP} + pv := ParamValue{Name: ctxP.Name, Path: ctxP.Path} if p.isBool { pv.Value = json.RawMessage("true") } else { @@ -159,17 +159,17 @@ func TestSources(t *T) { )) } -func TestSourceMap(t *T) { +func TestSourceParamValues(t *T) { ctx := mctx.New() a := RequiredInt(ctx, "a", "") foo := mctx.ChildOf(ctx, "foo") b := RequiredString(foo, "b", "") c := Bool(foo, "c", "") - err := Populate(ctx, SourceMap{ - "a": "4", - "foo-b": "bbb", - "foo-c": "1", + 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")}, }) massert.Fatal(t, massert.All( massert.Nil(err),