From 0385f6c201618390f1179e8b7776b21bd9e7dd38 Mon Sep 17 00:00:00 2001 From: Yashish Date: Fri, 21 Feb 2020 22:04:35 +0530 Subject: [PATCH 1/8] EncoderConfig and AtomicLevel validaton added --- config.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/config.go b/config.go index 6fe17d9e0..e1cd7087d 100644 --- a/config.go +++ b/config.go @@ -174,6 +174,13 @@ func (cfg Config) Build(opts ...Option) (*Logger, error) { return nil, err } + if cfg.Level == (AtomicLevel{}) { + cfg.Level = NewAtomicLevelAt(InfoLevel) + if cfg.Development { + cfg.Level = NewAtomicLevelAt(DebugLevel) + } + } + log := New( zapcore.NewCore(enc, sink, cfg.Level), cfg.buildOptions(errSink)..., @@ -239,5 +246,12 @@ func (cfg Config) openSinks() (zapcore.WriteSyncer, zapcore.WriteSyncer, error) } func (cfg Config) buildEncoder() (zapcore.Encoder, error) { + if cfg.EncoderConfig.TimeKey != "" && cfg.EncoderConfig.EncodeTime == nil { + cfg.EncoderConfig.EncodeTime = zapcore.EpochTimeEncoder + if cfg.Development { + cfg.EncoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder + } + } + return newEncoder(cfg.Encoding, cfg.EncoderConfig) } From 65887ac9937f0efb55eb0c25841ddfbf09837f12 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sat, 22 Feb 2020 08:24:24 -0800 Subject: [PATCH 2/8] config.go: Drop unnecessary parentheses --- config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.go b/config.go index e1cd7087d..a6e1819e7 100644 --- a/config.go +++ b/config.go @@ -174,7 +174,7 @@ func (cfg Config) Build(opts ...Option) (*Logger, error) { return nil, err } - if cfg.Level == (AtomicLevel{}) { + if cfg.Level == AtomicLevel{} { cfg.Level = NewAtomicLevelAt(InfoLevel) if cfg.Development { cfg.Level = NewAtomicLevelAt(DebugLevel) From dbd4283ef9bdc09ce18e2e3e903cb4da7ba76be9 Mon Sep 17 00:00:00 2001 From: Yashish Date: Sun, 1 Mar 2020 15:48:23 +0530 Subject: [PATCH 3/8] Return err instead default adding default values --- config.go | 11 +++-------- config_test.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/config.go b/config.go index e1cd7087d..14d86d406 100644 --- a/config.go +++ b/config.go @@ -21,6 +21,7 @@ package zap import ( + "fmt" "sort" "time" @@ -175,10 +176,7 @@ func (cfg Config) Build(opts ...Option) (*Logger, error) { } if cfg.Level == (AtomicLevel{}) { - cfg.Level = NewAtomicLevelAt(InfoLevel) - if cfg.Development { - cfg.Level = NewAtomicLevelAt(DebugLevel) - } + return nil, fmt.Errorf("missing Level") } log := New( @@ -247,10 +245,7 @@ func (cfg Config) openSinks() (zapcore.WriteSyncer, zapcore.WriteSyncer, error) func (cfg Config) buildEncoder() (zapcore.Encoder, error) { if cfg.EncoderConfig.TimeKey != "" && cfg.EncoderConfig.EncodeTime == nil { - cfg.EncoderConfig.EncodeTime = zapcore.EpochTimeEncoder - if cfg.Development { - cfg.EncoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder - } + return nil, fmt.Errorf("missing EncodeTime in EncoderConfig") } return newEncoder(cfg.Encoding, cfg.EncoderConfig) diff --git a/config_test.go b/config_test.go index 7a875703b..803bd89e3 100644 --- a/config_test.go +++ b/config_test.go @@ -27,6 +27,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/zap/zapcore" ) func TestConfig(t *testing.T) { @@ -106,3 +107,39 @@ func TestConfigWithInvalidPaths(t *testing.T) { }) } } + +func TestConfigWithMissingAttributes(t *testing.T) { + tests := []struct { + desc string + cfg Config + expectErr string + }{ + { + desc: "missing_level", + cfg: Config{ + Encoding: "json", + }, + expectErr: "missing Level", + }, + { + desc: "missing_encoder_time_in_encoder_config", + cfg: Config{ + Level: NewAtomicLevelAt(zapcore.InfoLevel), + Encoding: "json", + EncoderConfig: zapcore.EncoderConfig{ + MessageKey: "msg", + TimeKey: "ts", + }, + }, + expectErr: "missing EncodeTime in EncoderConfig", + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + cfg := tt.cfg + _, err := cfg.Build() + assert.Equal(t, err.Error(), tt.expectErr) + }) + } +} From 7744f0b16c6b2423f319f669757cb77634cc5ff8 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 3 Mar 2020 18:57:47 -0800 Subject: [PATCH 4/8] test: Fix testify ordering --- config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config_test.go b/config_test.go index 803bd89e3..cc7e377ee 100644 --- a/config_test.go +++ b/config_test.go @@ -139,7 +139,7 @@ func TestConfigWithMissingAttributes(t *testing.T) { t.Run(tt.desc, func(t *testing.T) { cfg := tt.cfg _, err := cfg.Build() - assert.Equal(t, err.Error(), tt.expectErr) + assert.Equal(t, tt.expectErr, err.Error()) }) } } From 70c2b473f2ee94e7e594bb1961aaed6142aa8efb Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 3 Mar 2020 18:58:13 -0800 Subject: [PATCH 5/8] test: Verify error is non-nil --- config_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/config_test.go b/config_test.go index cc7e377ee..9471e0aef 100644 --- a/config_test.go +++ b/config_test.go @@ -139,6 +139,7 @@ func TestConfigWithMissingAttributes(t *testing.T) { t.Run(tt.desc, func(t *testing.T) { cfg := tt.cfg _, err := cfg.Build() + require.Error(t, err) assert.Equal(t, tt.expectErr, err.Error()) }) } From 1e6b55bf5bc81519122f3c555468247ed8b22c04 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 3 Mar 2020 18:58:33 -0800 Subject: [PATCH 6/8] test: Remove underscores from test names --- config_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config_test.go b/config_test.go index 9471e0aef..0fa4b1efc 100644 --- a/config_test.go +++ b/config_test.go @@ -115,14 +115,14 @@ func TestConfigWithMissingAttributes(t *testing.T) { expectErr string }{ { - desc: "missing_level", + desc: "missing level", cfg: Config{ Encoding: "json", }, expectErr: "missing Level", }, { - desc: "missing_encoder_time_in_encoder_config", + desc: "missing encoder time in encoder config", cfg: Config{ Level: NewAtomicLevelAt(zapcore.InfoLevel), Encoding: "json", From 3f647e899d170939ccc4692046ec0d6d1e00bbe8 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 3 Mar 2020 19:03:27 -0800 Subject: [PATCH 7/8] Fix syntax error `if a == b{} { ... }` is syntactically invalid Go. --- config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.go b/config.go index c82424e96..14d86d406 100644 --- a/config.go +++ b/config.go @@ -175,7 +175,7 @@ func (cfg Config) Build(opts ...Option) (*Logger, error) { return nil, err } - if cfg.Level == AtomicLevel{} { + if cfg.Level == (AtomicLevel{}) { return nil, fmt.Errorf("missing Level") } From dffa12d4664d7a295d2091e02a3185b70ed1620e Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 3 Mar 2020 19:05:00 -0800 Subject: [PATCH 8/8] encoder: Validate EnxoderConfig in newEncoder --- config.go | 4 ---- encoder.go | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/config.go b/config.go index 14d86d406..eae1d237f 100644 --- a/config.go +++ b/config.go @@ -244,9 +244,5 @@ func (cfg Config) openSinks() (zapcore.WriteSyncer, zapcore.WriteSyncer, error) } func (cfg Config) buildEncoder() (zapcore.Encoder, error) { - if cfg.EncoderConfig.TimeKey != "" && cfg.EncoderConfig.EncodeTime == nil { - return nil, fmt.Errorf("missing EncodeTime in EncoderConfig") - } - return newEncoder(cfg.Encoding, cfg.EncoderConfig) } diff --git a/encoder.go b/encoder.go index 2e9d3c341..08ed83354 100644 --- a/encoder.go +++ b/encoder.go @@ -62,6 +62,10 @@ func RegisterEncoder(name string, constructor func(zapcore.EncoderConfig) (zapco } func newEncoder(name string, encoderConfig zapcore.EncoderConfig) (zapcore.Encoder, error) { + if encoderConfig.TimeKey != "" && encoderConfig.EncodeTime == nil { + return nil, fmt.Errorf("missing EncodeTime in EncoderConfig") + } + _encoderMutex.RLock() defer _encoderMutex.RUnlock() if name == "" {