From 8368d0881b1d79d3004b467eda31771183d6cff4 Mon Sep 17 00:00:00 2001
From: Brian Picciano <mediocregopher@gmail.com>
Date: Thu, 19 Jul 2018 18:43:17 +0000
Subject: [PATCH] mlog: change how Merge/MergeInto work, so they don't evaluate
 the KV immediately

---
 mlog/errctx.go    |  4 ++--
 mlog/mlog.go      | 37 ++++++++++++++++---------------------
 mlog/mlog_test.go | 16 ++++++++++++++--
 3 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/mlog/errctx.go b/mlog/errctx.go
index 51c8d26..da65c52 100644
--- a/mlog/errctx.go
+++ b/mlog/errctx.go
@@ -23,7 +23,7 @@ func ErrWithKV(err error, kvs ...KVer) merry.Error {
 	if exKV := merry.Value(merr, kvKey(0)); exKV != nil {
 		kv = mergeInto(exKV.(KV), kvs...)
 	} else {
-		kv = merge(kvs...)
+		kv = Merge(kvs...).KV()
 	}
 	return merr.WithValue(kvKey(0), kv)
 }
@@ -57,7 +57,7 @@ func CtxWithKV(ctx context.Context, kvs ...KVer) context.Context {
 	if existingKV != nil {
 		kv = mergeInto(existingKV.(KV), kvs...)
 	} else {
-		kv = merge(kvs...)
+		kv = Merge(kvs...).KV()
 	}
 	return context.WithValue(ctx, kvKey(0), kv)
 }
diff --git a/mlog/mlog.go b/mlog/mlog.go
index d95bbdf..dc32cbc 100644
--- a/mlog/mlog.go
+++ b/mlog/mlog.go
@@ -133,10 +133,6 @@ func (kv KV) Set(k string, v interface{}) KV {
 	return nkv
 }
 
-// separate from MergeInto because it's convenient to not return a KVer if that
-// KVer is going to immediately have KV called on it (and thereby create a copy
-// for no reason).
-//
 // this may take in any amount of nil values, but should never return nil
 func mergeInto(kv KVer, kvs ...KVer) KV {
 	if kv == nil {
@@ -154,32 +150,31 @@ func mergeInto(kv KVer, kvs ...KVer) KV {
 	return kvm
 }
 
-// separate from Merge because it's convenient to not return a KVer if that KVer
-// is going to immediately have KV called on it (and thereby create a copy for
-// no reason).
-//
-// this may take in any amount of nil values, but should never return nil
-func merge(kvs ...KVer) KV {
-	if len(kvs) == 0 {
-		return KV{}
-	}
-	return mergeInto(kvs[0], kvs[1:]...)
+type merger struct {
+	base KVer
+	rest []KVer
 }
 
 // Merge takes in multiple KVers and returns a single KVer which is the union of
 // all the passed in ones. Key/Vals on the rightmost of the set take precedence
 // over conflicting ones to the left.
 //
-// TODO currently this evaluates all of the KVers and generates a flat KV
-// instance in the moment. It would be better if this actually called the KV
-// methods of each KVer on every outer KV call.
+// The KVer returned will call KV() on each of the passed in KVers every time
+// its KV method is called.
 func Merge(kvs ...KVer) KVer {
-	return merge(kvs...)
+	if len(kvs) == 0 {
+		return merger{}
+	}
+	return merger{base: kvs[0], rest: kvs[1:]}
 }
 
 // MergeInto is a convenience function which acts similarly to Merge.
 func MergeInto(kv KVer, kvs ...KVer) KVer {
-	return mergeInto(kv, kvs...)
+	return merger{base: kv, rest: kvs}
+}
+
+func (m merger) KV() KV {
+	return mergeInto(m.base, m.rest...)
 }
 
 // Prefix prefixes the all keys returned from the given KVer with the given
@@ -252,7 +247,7 @@ type Logger struct {
 	wc       io.WriteCloser
 	wfn      WriteFn
 	maxLevel uint
-	kv       KV
+	kv       KVer
 
 	msgBufPool       *sync.Pool
 	msgCh            chan msg
@@ -328,7 +323,7 @@ func (l *Logger) WithWriteFn(wfn WriteFn) *Logger {
 // log messages.
 func (l *Logger) WithKV(kvs ...KVer) *Logger {
 	l = l.cp()
-	l.kv = mergeInto(l.kv, kvs...)
+	l.kv = MergeInto(l.kv, kvs...)
 	return l
 }
 
diff --git a/mlog/mlog_test.go b/mlog/mlog_test.go
index 7dddad1..4ed1f84 100644
--- a/mlog/mlog_test.go
+++ b/mlog/mlog_test.go
@@ -142,8 +142,7 @@ func TestDefaultWriteFn(t *T) {
 
 func TestMerge(t *T) {
 	assertMerge := func(exp KV, kvs ...KVer) massert.Assertion {
-		got := merge(kvs...)
-		return massert.Equal(exp, got)
+		return massert.Equal(exp, Merge(kvs...).KV())
 	}
 
 	massert.Fatal(t, massert.All(
@@ -168,6 +167,19 @@ func TestMerge(t *T) {
 			KV{"a": "a"}, KV{"a": "b"},
 		),
 	))
+
+	// Merge should _not_ call KV() on the inner KVers until the outer one is
+	// called.
+	{
+		kv := KV{"a": "a"}
+		mergedKV := Merge(kv)
+		kv["a"] = "b"
+		massert.Fatal(t, massert.All(
+			massert.Equal(KV{"a": "b"}, kv),
+			massert.Equal(KV{"a": "b"}, kv.KV()),
+			massert.Equal(KV{"a": "b"}, mergedKV.KV()),
+		))
+	}
 }
 
 func TestPrefix(t *T) {