mlog: change how Merge/MergeInto work, so they don't evaluate the KV immediately

This commit is contained in:
Brian Picciano 2018-07-19 18:43:17 +00:00
parent f0032a19d1
commit 8368d0881b
3 changed files with 32 additions and 25 deletions

View File

@ -23,7 +23,7 @@ func ErrWithKV(err error, kvs ...KVer) merry.Error {
if exKV := merry.Value(merr, kvKey(0)); exKV != nil { if exKV := merry.Value(merr, kvKey(0)); exKV != nil {
kv = mergeInto(exKV.(KV), kvs...) kv = mergeInto(exKV.(KV), kvs...)
} else { } else {
kv = merge(kvs...) kv = Merge(kvs...).KV()
} }
return merr.WithValue(kvKey(0), kv) return merr.WithValue(kvKey(0), kv)
} }
@ -57,7 +57,7 @@ func CtxWithKV(ctx context.Context, kvs ...KVer) context.Context {
if existingKV != nil { if existingKV != nil {
kv = mergeInto(existingKV.(KV), kvs...) kv = mergeInto(existingKV.(KV), kvs...)
} else { } else {
kv = merge(kvs...) kv = Merge(kvs...).KV()
} }
return context.WithValue(ctx, kvKey(0), kv) return context.WithValue(ctx, kvKey(0), kv)
} }

View File

@ -133,10 +133,6 @@ func (kv KV) Set(k string, v interface{}) KV {
return nkv 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 // this may take in any amount of nil values, but should never return nil
func mergeInto(kv KVer, kvs ...KVer) KV { func mergeInto(kv KVer, kvs ...KVer) KV {
if kv == nil { if kv == nil {
@ -154,32 +150,31 @@ func mergeInto(kv KVer, kvs ...KVer) KV {
return kvm return kvm
} }
// separate from Merge because it's convenient to not return a KVer if that KVer type merger struct {
// is going to immediately have KV called on it (and thereby create a copy for base KVer
// no reason). rest []KVer
//
// 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:]...)
} }
// Merge takes in multiple KVers and returns a single KVer which is the union of // 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 // all the passed in ones. Key/Vals on the rightmost of the set take precedence
// over conflicting ones to the left. // over conflicting ones to the left.
// //
// TODO currently this evaluates all of the KVers and generates a flat KV // The KVer returned will call KV() on each of the passed in KVers every time
// instance in the moment. It would be better if this actually called the KV // its KV method is called.
// methods of each KVer on every outer KV call.
func Merge(kvs ...KVer) KVer { 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. // MergeInto is a convenience function which acts similarly to Merge.
func MergeInto(kv KVer, kvs ...KVer) KVer { 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 // Prefix prefixes the all keys returned from the given KVer with the given
@ -252,7 +247,7 @@ type Logger struct {
wc io.WriteCloser wc io.WriteCloser
wfn WriteFn wfn WriteFn
maxLevel uint maxLevel uint
kv KV kv KVer
msgBufPool *sync.Pool msgBufPool *sync.Pool
msgCh chan msg msgCh chan msg
@ -328,7 +323,7 @@ func (l *Logger) WithWriteFn(wfn WriteFn) *Logger {
// log messages. // log messages.
func (l *Logger) WithKV(kvs ...KVer) *Logger { func (l *Logger) WithKV(kvs ...KVer) *Logger {
l = l.cp() l = l.cp()
l.kv = mergeInto(l.kv, kvs...) l.kv = MergeInto(l.kv, kvs...)
return l return l
} }

View File

@ -142,8 +142,7 @@ func TestDefaultWriteFn(t *T) {
func TestMerge(t *T) { func TestMerge(t *T) {
assertMerge := func(exp KV, kvs ...KVer) massert.Assertion { assertMerge := func(exp KV, kvs ...KVer) massert.Assertion {
got := merge(kvs...) return massert.Equal(exp, Merge(kvs...).KV())
return massert.Equal(exp, got)
} }
massert.Fatal(t, massert.All( massert.Fatal(t, massert.All(
@ -168,6 +167,19 @@ func TestMerge(t *T) {
KV{"a": "a"}, KV{"a": "b"}, 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) { func TestPrefix(t *T) {