From be17ee942a077e95ba0926ed845cc085d8949452 Mon Sep 17 00:00:00 2001
From: Brian Picciano <mediocregopher@gmail.com>
Date: Mon, 13 Aug 2018 20:05:22 -0400
Subject: [PATCH] mcfg: simplify populateParams

---
 mcfg/cli.go    |  8 ++-----
 mcfg/mcfg.go   | 57 ++++++++------------------------------------------
 mcfg/source.go | 23 +++++++++++++++++++-
 3 files changed, 33 insertions(+), 55 deletions(-)

diff --git a/mcfg/cli.go b/mcfg/cli.go
index 8aca3c2..320fecd 100644
--- a/mcfg/cli.go
+++ b/mcfg/cli.go
@@ -123,12 +123,8 @@ func (cli SourceCLI) Parse(cfg *Cfg) ([]ParamValue, error) {
 func (cli SourceCLI) cliParamVals(cfg *Cfg) (map[string]ParamValue, error) {
 	m := map[string]ParamValue{}
 	for _, pv := range cfg.allParamValues() {
-		key := cliKeyPrefix
-		if len(pv.Path) > 0 {
-			key += strings.Join(pv.Path, cliKeyJoin) + cliKeyJoin
-		}
-		key += pv.Param.Name
-		m[key] = pv
+		key := strings.Join(append(pv.Path, pv.Param.Name), cliKeyJoin)
+		m[cliKeyPrefix+key] = pv
 	}
 	return m, nil
 }
diff --git a/mcfg/mcfg.go b/mcfg/mcfg.go
index 0d70100..abc4a42 100644
--- a/mcfg/mcfg.go
+++ b/mcfg/mcfg.go
@@ -140,62 +140,23 @@ func (c *Cfg) populateParams(src Source) error {
 		}
 	}
 
-	// first dedupe the params. We use this param struct as the key by which to
-	// dedupe by. Its use depends on the json.Marshaler always ordering fields
-	// in a marshaled struct the same way, which isn't the best assumption but
-	// it's ok for now
-	type param struct {
-		Path []string `json:",omitempty"`
-		Name string
-	}
-	paramFullName := func(p param) string {
-		if len(p.Path) == 0 {
-			return p.Name
-		}
-		slice := append(make([]string, 0, len(p.Path)+1), p.Name)
-		slice = append(slice, p.Path...)
-		return strings.Join(slice, "-")
-	}
-
+	// dedupe the ParamValues based on their hashes, with the last ParamValue
+	// taking precedence
 	pvM := map[string]ParamValue{}
 	for _, pv := range pvs {
-		keyB, err := json.Marshal(param{Path: pv.Path, Name: pv.Name})
-		if err != nil {
-			return err
-		}
-		pvM[string(keyB)] = pv
+		pvM[pv.hash()] = pv
 	}
 
-	// check for required params, again using the param struct and the existing
-	// pvM
-	var requiredParams func(*Cfg) []param
-	requiredParams = func(c *Cfg) []param {
-		var out []param
-		for _, p := range c.Params {
-			if !p.Required {
-				continue
-			}
-			out = append(out, param{Path: c.Path, Name: p.Name})
-		}
-		for _, child := range c.Children {
-			out = append(out, requiredParams(child)...)
-		}
-		return out
-	}
-
-	for _, reqP := range requiredParams(c) {
-		keyB, err := json.Marshal(reqP)
-		if err != nil {
-			return err
-		} else if _, ok := pvM[string(keyB)]; !ok {
-			return fmt.Errorf("param %s is required but wasn't populated by any configuration source", paramFullName(reqP))
+	// check for required params
+	for _, pv := range c.allParamValues() {
+		if !pv.Param.Required {
+			continue
+		} else if _, ok := pvM[pv.hash()]; !ok {
+			return fmt.Errorf("param %q is required", pv.displayName())
 		}
 	}
 
 	for _, pv := range pvM {
-		if pv.Into == nil {
-			continue
-		}
 		if err := json.Unmarshal(pv.Value, pv.Into); err != nil {
 			return err
 		}
diff --git a/mcfg/source.go b/mcfg/source.go
index 185d9c9..8f334e0 100644
--- a/mcfg/source.go
+++ b/mcfg/source.go
@@ -1,6 +1,12 @@
 package mcfg
 
-import "encoding/json"
+import (
+	"crypto/md5"
+	"encoding/hex"
+	"encoding/json"
+	"fmt"
+	"strings"
+)
 
 // ParamValue describes a value for a parameter which has been parsed by a
 // Source
@@ -10,6 +16,21 @@ type ParamValue struct {
 	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 {