Skip to content

Commit 0251e7b

Browse files
committed
Fix pointer references in ctxlog
Signed-off-by: Carlos Martín <carlos.martin.sanchez@gmail.com>
1 parent e85def2 commit 0251e7b

File tree

2 files changed

+109
-9
lines changed

2 files changed

+109
-9
lines changed

util/ctxlog/context.go

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,17 @@ func Get(ctx context.Context) log.Logger {
2525
return NewLogger(fields)
2626
}
2727

28-
// Fields returns the context log fields. It can be nil
28+
// Fields returns a copy of the context log fields. It can be nil
2929
func Fields(ctx context.Context) log.Fields {
3030
if v := ctx.Value(logFieldsKey); v != nil {
31-
return v.(log.Fields)
31+
f := v.(log.Fields)
32+
copy := make(map[string]interface{}, len(f))
33+
34+
for key, val := range f {
35+
copy[key] = val
36+
}
37+
38+
return copy
3239
}
3340

3441
return nil
@@ -41,16 +48,16 @@ func WithLogFields(ctx context.Context, fields log.Fields) (context.Context, log
4148
return ctx, Get(ctx)
4249
}
4350

44-
var newFields log.Fields
51+
newFields := make(map[string]interface{}, len(fields))
4552

4653
if v := ctx.Value(logFieldsKey); v != nil {
47-
newFields = v.(log.Fields)
48-
49-
for k, v := range fields {
50-
newFields[k] = v
54+
for key, val := range v.(log.Fields) {
55+
newFields[key] = val
5156
}
52-
} else {
53-
newFields = fields
57+
}
58+
59+
for key, val := range fields {
60+
newFields[key] = val
5461
}
5562

5663
ctx = set(ctx, newFields)

util/ctxlog/context_test.go

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
package ctxlog_test
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
log "gopkg.in/src-d/go-log.v1"
9+
10+
"github.com/src-d/lookout/util/ctxlog"
11+
)
12+
13+
func TestEmptyGet(t *testing.T) {
14+
ctx := context.Background()
15+
16+
log := ctxlog.Get(ctx)
17+
18+
require.NotNil(t, log)
19+
20+
require.NotPanics(t, func() {
21+
log.Debugf("a test log msg")
22+
})
23+
}
24+
25+
func TestNilFields(t *testing.T) {
26+
ctx := context.Background()
27+
28+
fields := ctxlog.Fields(ctx)
29+
30+
require.Nil(t, fields)
31+
}
32+
33+
func TestWithLogFields(t *testing.T) {
34+
ctxA := context.Background()
35+
ctxAB, _ := ctxlog.WithLogFields(ctxA, log.Fields{"keyB": "valB"})
36+
ctxABC, _ := ctxlog.WithLogFields(ctxAB, log.Fields{"keyC": "valC"})
37+
ctxABD, _ := ctxlog.WithLogFields(ctxAB, log.Fields{"keyD": "valD"})
38+
39+
fieldsA := ctxlog.Fields(ctxA)
40+
fieldsB := ctxlog.Fields(ctxAB)
41+
fieldsC := ctxlog.Fields(ctxABC)
42+
fieldsD := ctxlog.Fields(ctxABD)
43+
44+
require.NotEqual(t, fieldsA, fieldsB)
45+
require.NotEqual(t, fieldsB, fieldsC)
46+
47+
require.Nil(t, fieldsA["keyB"])
48+
require.Nil(t, fieldsA["keyC"])
49+
require.Nil(t, fieldsA["keyD"])
50+
51+
require.Equal(t, "valB", fieldsB["keyB"])
52+
require.Nil(t, fieldsB["keyC"])
53+
require.Nil(t, fieldsB["keyD"])
54+
55+
require.Equal(t, "valB", fieldsC["keyB"])
56+
require.Equal(t, "valC", fieldsC["keyC"])
57+
require.Nil(t, fieldsC["keyD"])
58+
59+
require.Equal(t, "valB", fieldsD["keyB"])
60+
require.Nil(t, fieldsD["keyC"])
61+
require.Equal(t, "valD", fieldsD["keyD"])
62+
}
63+
64+
func TestWithNilLogFields(t *testing.T) {
65+
ctxA := context.Background()
66+
67+
ctxB, logger := ctxlog.WithLogFields(ctxA, nil)
68+
69+
require.NotNil(t, ctxB)
70+
require.NotNil(t, logger)
71+
}
72+
73+
func TestReadOnlyWithLogFields(t *testing.T) {
74+
ctxA := context.Background()
75+
ctxB, _ := ctxlog.WithLogFields(ctxA, log.Fields{"keyB": "valB"})
76+
77+
fieldsA := ctxlog.Fields(ctxA)
78+
fieldsB := ctxlog.Fields(ctxB)
79+
80+
require.Nil(t, fieldsA["keyB"])
81+
require.Equal(t, "valB", fieldsB["keyB"])
82+
83+
fieldsB["keyY"] = "valY"
84+
85+
fieldsA2 := ctxlog.Fields(ctxA)
86+
fieldsB2 := ctxlog.Fields(ctxB)
87+
88+
require.Nil(t, fieldsA2["keyB"])
89+
require.Nil(t, fieldsA2["keyY"])
90+
91+
require.Equal(t, "valB", fieldsB2["keyB"])
92+
require.Nil(t, fieldsB2["keyY"])
93+
}

0 commit comments

Comments
 (0)