From 132c51eaf0001ebb2da60e98ea12cf3907e58198 Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Mon, 13 Aug 2018 21:02:06 -0400 Subject: [PATCH] mcfg: move Path out of ParamValue and into Param, which allows for shuffling around a whole bunch of code to make things make a bit more sense --- mcfg/cli.go | 46 +++++++++++++++++----------------- mcfg/cli_test.go | 4 +-- mcfg/env.go | 16 ++++++------ mcfg/mcfg.go | 20 ++++++--------- mcfg/param.go | 49 ++++++++++++++++++++++++++++++++++++ mcfg/source.go | 61 +++++---------------------------------------- mcfg/source_test.go | 6 ++--- 7 files changed, 98 insertions(+), 104 deletions(-) diff --git a/mcfg/cli.go b/mcfg/cli.go index dd39be8..9a9ced6 100644 --- a/mcfg/cli.go +++ b/mcfg/cli.go @@ -47,7 +47,7 @@ func (cli SourceCLI) Parse(cfg *Cfg) ([]ParamValue, error) { args = os.Args[1:] } - pvM, err := cli.cliParamVals(cfg) + pM, err := cli.cliParams(cfg) if err != nil { return nil, err } @@ -64,11 +64,11 @@ func (cli SourceCLI) Parse(cfg *Cfg) ([]ParamValue, error) { pvStrVal = arg pvStrValOk = true } else if !cli.DisableHelpPage && arg == cliHelpArg { - cli.printHelp(os.Stdout, pvM) + cli.printHelp(os.Stdout, pM) os.Stdout.Sync() os.Exit(1) } else { - for key, pv = range pvM { + for key, pv.Param = range pM { if arg == key { pvOk = true break @@ -102,7 +102,7 @@ func (cli SourceCLI) Parse(cfg *Cfg) ([]ParamValue, error) { continue } - pv.Value = fuzzyParse(pv.Param, pvStrVal) + pv.Value = pv.Param.fuzzyParse(pvStrVal) pvs = append(pvs, pv) key = "" @@ -117,28 +117,28 @@ func (cli SourceCLI) Parse(cfg *Cfg) ([]ParamValue, error) { return pvs, nil } -func (cli SourceCLI) cliParamVals(cfg *Cfg) (map[string]ParamValue, error) { - m := map[string]ParamValue{} - for _, pv := range cfg.allParamValues() { - key := strings.Join(append(pv.Path, pv.Param.Name), cliKeyJoin) - m[cliKeyPrefix+key] = pv +func (cli SourceCLI) cliParams(cfg *Cfg) (map[string]Param, error) { + m := map[string]Param{} + for _, p := range cfg.allParams() { + key := strings.Join(append(p.Path, p.Name), cliKeyJoin) + m[cliKeyPrefix+key] = p } return m, nil } -func (cli SourceCLI) printHelp(w io.Writer, pvM map[string]ParamValue) { - type pvEntry struct { +func (cli SourceCLI) printHelp(w io.Writer, pM map[string]Param) { + type pEntry struct { arg string - ParamValue + Param } - pvA := make([]pvEntry, 0, len(pvM)) - for arg, pv := range pvM { - pvA = append(pvA, pvEntry{arg: arg, ParamValue: pv}) + pA := make([]pEntry, 0, len(pM)) + for arg, p := range pM { + pA = append(pA, pEntry{arg: arg, Param: p}) } - sort.Slice(pvA, func(i, j int) bool { - return pvA[i].arg < pvA[j].arg + sort.Slice(pA, func(i, j int) bool { + return pA[i].arg < pA[j].arg }) fmtDefaultVal := func(ptr interface{}) string { @@ -155,16 +155,16 @@ func (cli SourceCLI) printHelp(w io.Writer, pvM map[string]ParamValue) { return fmt.Sprint(val.Interface()) } - for _, pvE := range pvA { - fmt.Fprintf(w, "\n%s", pvE.arg) - if pvE.IsBool { + for _, p := range pA { + fmt.Fprintf(w, "\n%s", p.arg) + if p.IsBool { fmt.Fprintf(w, " (Flag)") - } else if defVal := fmtDefaultVal(pvE.Into); defVal != "" { + } else if defVal := fmtDefaultVal(p.Into); defVal != "" { fmt.Fprintf(w, " (Default: %s)", defVal) } fmt.Fprintf(w, "\n") - if pvE.Usage != "" { - fmt.Fprintln(w, "\t"+pvE.Usage) + if p.Usage != "" { + fmt.Fprintln(w, "\t"+p.Usage) } } fmt.Fprintf(w, "\n") diff --git a/mcfg/cli_test.go b/mcfg/cli_test.go index 94422df..50db500 100644 --- a/mcfg/cli_test.go +++ b/mcfg/cli_test.go @@ -21,9 +21,9 @@ func TestSourceCLIHelp(t *T) { src := SourceCLI{} buf := new(bytes.Buffer) - pvM, err := src.cliParamVals(cfg) + pM, err := src.cliParams(cfg) require.NoError(t, err) - SourceCLI{}.printHelp(buf, pvM) + SourceCLI{}.printHelp(buf, pM) exp := ` --bar (Flag) diff --git a/mcfg/env.go b/mcfg/env.go index 588960a..0b6ce3b 100644 --- a/mcfg/env.go +++ b/mcfg/env.go @@ -43,10 +43,10 @@ func (env SourceEnv) Parse(cfg *Cfg) ([]ParamValue, error) { kvs = os.Environ() } - pvM := map[string]ParamValue{} - for _, pv := range cfg.allParamValues() { - name := env.expectedName(pv.Path, pv.Name) - pvM[name] = pv + pM := map[string]Param{} + for _, p := range cfg.allParams() { + name := env.expectedName(p.Path, p.Name) + pM[name] = p } pvs := make([]ParamValue, 0, len(kvs)) @@ -56,9 +56,11 @@ func (env SourceEnv) Parse(cfg *Cfg) ([]ParamValue, error) { return nil, fmt.Errorf("malformed environment kv %q", kv) } k, v := split[0], split[1] - if pv, ok := pvM[k]; ok { - pv.Value = fuzzyParse(pv.Param, v) - pvs = append(pvs, pv) + if p, ok := pM[k]; ok { + pvs = append(pvs, ParamValue{ + Param: p, + Value: p.fuzzyParse(v), + }) } } diff --git a/mcfg/mcfg.go b/mcfg/mcfg.go index abc4a42..5a0da5e 100644 --- a/mcfg/mcfg.go +++ b/mcfg/mcfg.go @@ -10,7 +10,6 @@ import ( ) // TODO Sources: -// - Env // - Env file // - JSON file // - YAML file @@ -130,14 +129,9 @@ func (c *Cfg) FullName() string { } func (c *Cfg) populateParams(src Source) error { - // we allow for nil Source here for tests - // TODO make Source stub type which tests could use here instead - var pvs []ParamValue - if src != nil { - var err error - if pvs, err = src.Parse(c); err != nil { - return err - } + pvs, err := src.Parse(c) + if err != nil { + return err } // dedupe the ParamValues based on their hashes, with the last ParamValue @@ -148,11 +142,11 @@ func (c *Cfg) populateParams(src Source) error { } // check for required params - for _, pv := range c.allParamValues() { - if !pv.Param.Required { + for _, p := range c.allParams() { + if !p.Required { continue - } else if _, ok := pvM[pv.hash()]; !ok { - return fmt.Errorf("param %q is required", pv.displayName()) + } else if _, ok := pvM[p.hash()]; !ok { + return fmt.Errorf("param %q is required", p.fullName()) } } diff --git a/mcfg/param.go b/mcfg/param.go index 4bca026..b8476f9 100644 --- a/mcfg/param.go +++ b/mcfg/param.go @@ -1,6 +1,9 @@ package mcfg import ( + "crypto/md5" + "encoding/hex" + "encoding/json" "fmt" "strings" @@ -36,6 +39,39 @@ type Param struct { // json.Unmarshal'd. The value being pointed to also determines the default // value of the parameter. Into interface{} + + // The Path field of the Cfg this Param is attached to. NOTE that this + // will be automatically filled in when the Param is added to the Cfg. + Path []string +} + +func (p Param) fuzzyParse(v string) json.RawMessage { + if p.IsBool { + if v == "" || v == "0" || v == "false" { + return json.RawMessage("false") + } + return json.RawMessage("true") + + } else if p.IsString && (v == "" || v[0] != '"') { + return json.RawMessage(`"` + v + `"`) + } + + 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 } // ParamAdd adds the given Param to the Cfg. It will panic if a Param of the @@ -45,9 +81,22 @@ func (c *Cfg) ParamAdd(p Param) { if _, ok := c.Params[p.Name]; ok { panic(fmt.Sprintf("Cfg.Path:%#v name:%q already exists", c.Path, p.Name)) } + p.Path = c.Path c.Params[p.Name] = p } +func (c *Cfg) allParams() []Param { + params := make([]Param, 0, len(c.Params)) + for _, p := range c.Params { + params = append(params, p) + } + + for _, child := range c.Children { + params = append(params, child.allParams()...) + } + return params +} + // ParamInt64 returns an *int64 which will be populated once the Cfg is run func (c *Cfg) ParamInt64(name string, defaultVal int64, usage string) *int64 { i := defaultVal diff --git a/mcfg/source.go b/mcfg/source.go index ebcb4e7..959f210 100644 --- a/mcfg/source.go +++ b/mcfg/source.go @@ -1,67 +1,16 @@ package mcfg import ( - "crypto/md5" - "encoding/hex" "encoding/json" - "fmt" - "strings" ) -func fuzzyParse(p Param, v string) json.RawMessage { - if p.IsBool { - if v == "" || v == "0" || v == "false" { - return json.RawMessage("false") - } - return json.RawMessage("true") - - } else if p.IsString && (v == "" || v[0] != '"') { - return json.RawMessage(`"` + v + `"`) - } - - return json.RawMessage(v) -} - -// TODO moving Path into Param would make a lot more sense - // ParamValue describes a value for a parameter which has been parsed by a // Source type ParamValue struct { Param - Path []string // nil if root Value json.RawMessage } -func (pv ParamValue) displayName() string { - return strings.Join(append(pv.Path, pv.Param.Name), "-") -} - -func (pv ParamValue) hash() string { - h := md5.New() - for _, path := range pv.Path { - fmt.Fprintf(h, "pathEl:%q\n", path) - } - fmt.Fprintf(h, "name:%q\n", pv.Param.Name) - hStr := hex.EncodeToString(h.Sum(nil)) - // we add the displayName to it to make debugging easier - return pv.displayName() + "/" + hStr -} - -func (cfg *Cfg) allParamValues() []ParamValue { - pvs := make([]ParamValue, 0, len(cfg.Params)) - for _, param := range cfg.Params { - pvs = append(pvs, ParamValue{ - Param: param, - Path: cfg.Path, - }) - } - - for _, child := range cfg.Children { - pvs = append(pvs, child.allParamValues()...) - } - return pvs -} - // Source parses ParamValues out of a particular configuration source. The // returned []ParamValue may contain duplicates of the same Param's value. type Source interface { @@ -94,10 +43,12 @@ type SourceMap map[string]string func (m SourceMap) Parse(c *Cfg) ([]ParamValue, error) { pvs := make([]ParamValue, 0, len(m)) - for _, pv := range c.allParamValues() { - if v, ok := m[pv.displayName()]; ok { - pv.Value = fuzzyParse(pv.Param, v) - pvs = append(pvs, pv) + for _, p := range c.allParams() { + 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 062b935..5591514 100644 --- a/mcfg/source_test.go +++ b/mcfg/source_test.go @@ -105,12 +105,10 @@ func (scs srcCommonState) applyCfgAndPV(p srcCommonParams) srcCommonState { // those are only used by Cfg once it has all ParamValues together } thisCfg.ParamAdd(cfgP) + cfgP = thisCfg.Params[p.name] // get it back out to get any added fields if !p.unset { - pv := ParamValue{ - Param: cfgP, - Path: p.path, - } + pv := ParamValue{Param: cfgP} if p.isBool { pv.Value = json.RawMessage("true") } else {