mcfg: make ParamValue not embed Param, so that a Source which is []ParamValue makes sense, and can replace SourceMap

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.New("unexpected config parameter", "param", arg)
-				return nil, merr.WithValue(err, "param", arg, true)
 			}
 		}
 
@@ -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.New("param expected a value", "param", key)
-		return nil, merr.WithValue(err, "param", key, true)
 	}
 	return pvs, nil
 }

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.New("malformed environment key/value pair", "kv", kv)
-			return nil, merr.WithValue(err, "kv", kv, true)
 		}
 		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),
 			})
 		}

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 {
+	for hash, p := range pM {
-		if !param.Required {
+		if !p.Required {
 			continue
-		} else if _, ok := pvM[param.hash()]; !ok {
+		} else if _, ok := pvM[hash]; !ok {
-			err := merr.New("required parameter is not set")
+			return merr.New("required parameter is not set",
-			return merr.WithValue(err, "param", param.fullName(), true)
+				"param", paramFullName(p.Path, p.Name))
 		}
 	}
 
-	for _, pv := range pvM {
+	// do the actual populating
-		if err := json.Unmarshal(pv.Value, pv.Into); err != nil {
+	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
 		}
 	}

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) {

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
-}

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{
+	err := Populate(ctx, ParamValues{
-		"a":     "4",
+		{Name: "a", Value: json.RawMessage(`4`)},
-		"foo-b": "bbb",
+		{Path: []string{"foo"}, Name: "b", Value: json.RawMessage(`"bbb"`)},
-		"foo-c": "1",
+		{Path: []string{"foo"}, Name: "c", Value: json.RawMessage("true")},
 	})
 	massert.Fatal(t, massert.All(
 		massert.Nil(err),