From e2fb3f696e5c5f4589b33ccc5c7f3d258b380848 Mon Sep 17 00:00:00 2001 From: Peter Edge Date: Mon, 6 Mar 2017 18:26:38 -0500 Subject: [PATCH 1/4] Add RegisterEncoder functionality --- config.go | 9 +---- encoder.go | 84 ++++++++++++++++++++++++++++++++++++++++++++ encoder_test.go | 92 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 177 insertions(+), 8 deletions(-) create mode 100644 encoder.go create mode 100644 encoder_test.go diff --git a/config.go b/config.go index 0709fc25e..099d1b34c 100644 --- a/config.go +++ b/config.go @@ -21,7 +21,6 @@ package zap import ( - "fmt" "sort" "time" @@ -229,11 +228,5 @@ func (cfg Config) openSinks() (zapcore.WriteSyncer, zapcore.WriteSyncer, error) } func (cfg Config) buildEncoder() (zapcore.Encoder, error) { - switch cfg.Encoding { - case "json": - return zapcore.NewJSONEncoder(cfg.EncoderConfig), nil - case "console": - return zapcore.NewConsoleEncoder(cfg.EncoderConfig), nil - } - return nil, fmt.Errorf("unknown encoding %q", cfg.Encoding) + return newEncoder(cfg.Encoding, cfg.EncoderConfig) } diff --git a/encoder.go b/encoder.go new file mode 100644 index 000000000..0cc0e1dc6 --- /dev/null +++ b/encoder.go @@ -0,0 +1,84 @@ +// Copyright (c) 2016 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package zap + +import ( + "errors" + "fmt" + "sync" + + "go.uber.org/zap/zapcore" +) + +var ( + errNoEncoderNameSpecified = errors.New("no encoder name specified ") + encoderNameToConstructor = make(map[string]func(zapcore.EncoderConfig) zapcore.Encoder) + encoderMutex sync.RWMutex +) + +func init() { + registerDefaultEncoders() +} + +// RegisterEncoder registers an encoder constructor for the given name. +// +// If an encoder with the same name already exists, this will panic. +// By default, the encoders "json" and "console" are registered. +func MustRegisterEncoder(name string, constructor func(zapcore.EncoderConfig) zapcore.Encoder) { + if err := RegisterEncoder(name, constructor); err != nil { + panic(err.Error()) + } +} + +// RegisterEncoder registers an encoder constructor for the given name. +// +// If an encoder with the same name already exists, this will return an error. +// By default, the encoders "json" and "console" are registered. +func RegisterEncoder(name string, constructor func(zapcore.EncoderConfig) zapcore.Encoder) error { + encoderMutex.Lock() + defer encoderMutex.Unlock() + if name == "" { + return errNoEncoderNameSpecified + } + if _, ok := encoderNameToConstructor[name]; ok { + return fmt.Errorf("encoder already registered for name %s", name) + } + encoderNameToConstructor[name] = constructor + return nil +} + +func registerDefaultEncoders() { + MustRegisterEncoder("console", zapcore.NewConsoleEncoder) + MustRegisterEncoder("json", zapcore.NewJSONEncoder) +} + +func newEncoder(name string, encoderConfig zapcore.EncoderConfig) (zapcore.Encoder, error) { + encoderMutex.RLock() + defer encoderMutex.RUnlock() + if name == "" { + return nil, errNoEncoderNameSpecified + } + constructor, ok := encoderNameToConstructor[name] + if !ok { + return nil, fmt.Errorf("no encoder registered for name %s", name) + } + return constructor(encoderConfig), nil +} diff --git a/encoder_test.go b/encoder_test.go new file mode 100644 index 000000000..5d22f9a35 --- /dev/null +++ b/encoder_test.go @@ -0,0 +1,92 @@ +// Copyright (c) 2016 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package zap + +import ( + "testing" + + "go.uber.org/zap/zapcore" + + "github.com/stretchr/testify/assert" +) + +var ( + newNilEncoder = func(_ zapcore.EncoderConfig) zapcore.Encoder { + return nil + } +) + +func TestRegisterDefaultEncoders(t *testing.T) { + testEncodersRegistered(t, "console", "json") +} + +func TestRegisterEncoder(t *testing.T) { + testEncoders(func() { + assert.NoError(t, RegisterEncoder("foo", newNilEncoder)) + testEncodersRegistered(t, "foo") + }) +} + +func TestDuplicateRegisterEncoder(t *testing.T) { + testEncoders(func() { + RegisterEncoder("foo", newNilEncoder) + assert.Error(t, RegisterEncoder("foo", newNilEncoder)) + }) +} + +func TestRegisterEncoderNoName(t *testing.T) { + assert.Equal(t, errNoEncoderNameSpecified, RegisterEncoder("", newNilEncoder)) +} + +func TestNewEncoder(t *testing.T) { + testEncoders(func() { + RegisterEncoder("foo", newNilEncoder) + encoder, err := newEncoder("foo", zapcore.EncoderConfig{}) + assert.NoError(t, err) + assert.Nil(t, encoder) + }) +} + +func TestNewEncoderNotRegistered(t *testing.T) { + _, err := newEncoder("foo", zapcore.EncoderConfig{}) + assert.Error(t, err) +} + +func TestNewEncoderNoName(t *testing.T) { + _, err := newEncoder("", zapcore.EncoderConfig{}) + assert.Equal(t, errNoEncoderNameSpecified, err) +} + +func testEncoders(f func()) { + encoderNameToConstructor = make(map[string]func(zapcore.EncoderConfig) zapcore.Encoder) + defer func() { + encoderNameToConstructor = make(map[string]func(zapcore.EncoderConfig) zapcore.Encoder) + registerDefaultEncoders() + }() + f() +} + +func testEncodersRegistered(t *testing.T, names ...string) { + assert.Len(t, encoderNameToConstructor, len(names)) + for _, name := range names { + assert.NotNil(t, encoderNameToConstructor[name]) + } +} From 79830b92ce5f1f38193ecd908a5371840c6bb501 Mon Sep 17 00:00:00 2001 From: Peter Edge Date: Mon, 6 Mar 2017 18:30:06 -0500 Subject: [PATCH 2/4] Add test for MustRegisterEncoder --- encoder_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/encoder_test.go b/encoder_test.go index 5d22f9a35..f7bcf840a 100644 --- a/encoder_test.go +++ b/encoder_test.go @@ -52,6 +52,15 @@ func TestDuplicateRegisterEncoder(t *testing.T) { }) } +func TestDuplicateMustRegisterEncoder(t *testing.T) { + testEncoders(func() { + RegisterEncoder("foo", newNilEncoder) + assert.Panics(t, func() { + MustRegisterEncoder("foo", newNilEncoder) + }) + }) +} + func TestRegisterEncoderNoName(t *testing.T) { assert.Equal(t, errNoEncoderNameSpecified, RegisterEncoder("", newNilEncoder)) } From f7f1fdc02012651284cb34a4d186decdc33ccea1 Mon Sep 17 00:00:00 2001 From: Peter Edge Date: Mon, 6 Mar 2017 19:15:23 -0500 Subject: [PATCH 3/4] Fix make lint --- encoder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/encoder.go b/encoder.go index 0cc0e1dc6..6b9812545 100644 --- a/encoder.go +++ b/encoder.go @@ -38,7 +38,7 @@ func init() { registerDefaultEncoders() } -// RegisterEncoder registers an encoder constructor for the given name. +// MustRegisterEncoder registers an encoder constructor for the given name. // // If an encoder with the same name already exists, this will panic. // By default, the encoders "json" and "console" are registered. From e3b9d4fef6683dbe41ed27ba71dbdb43da0565e4 Mon Sep 17 00:00:00 2001 From: Peter Edge Date: Wed, 8 Mar 2017 19:34:19 -0500 Subject: [PATCH 4/4] Update per comments --- encoder.go | 22 ++++++++++++++-------- encoder_test.go | 18 +++++++----------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/encoder.go b/encoder.go index 6b9812545..7e65af3cc 100644 --- a/encoder.go +++ b/encoder.go @@ -30,7 +30,7 @@ import ( var ( errNoEncoderNameSpecified = errors.New("no encoder name specified ") - encoderNameToConstructor = make(map[string]func(zapcore.EncoderConfig) zapcore.Encoder) + encoderNameToConstructor = make(map[string]func(zapcore.EncoderConfig) (zapcore.Encoder, error)) encoderMutex sync.RWMutex ) @@ -42,7 +42,7 @@ func init() { // // If an encoder with the same name already exists, this will panic. // By default, the encoders "json" and "console" are registered. -func MustRegisterEncoder(name string, constructor func(zapcore.EncoderConfig) zapcore.Encoder) { +func MustRegisterEncoder(name string, constructor func(zapcore.EncoderConfig) (zapcore.Encoder, error)) { if err := RegisterEncoder(name, constructor); err != nil { panic(err.Error()) } @@ -52,22 +52,22 @@ func MustRegisterEncoder(name string, constructor func(zapcore.EncoderConfig) za // // If an encoder with the same name already exists, this will return an error. // By default, the encoders "json" and "console" are registered. -func RegisterEncoder(name string, constructor func(zapcore.EncoderConfig) zapcore.Encoder) error { +func RegisterEncoder(name string, constructor func(zapcore.EncoderConfig) (zapcore.Encoder, error)) error { encoderMutex.Lock() defer encoderMutex.Unlock() if name == "" { return errNoEncoderNameSpecified } if _, ok := encoderNameToConstructor[name]; ok { - return fmt.Errorf("encoder already registered for name %s", name) + return fmt.Errorf("encoder already registered for name %q", name) } encoderNameToConstructor[name] = constructor return nil } func registerDefaultEncoders() { - MustRegisterEncoder("console", zapcore.NewConsoleEncoder) - MustRegisterEncoder("json", zapcore.NewJSONEncoder) + MustRegisterEncoder("console", wrapEncoderConstructorNoError(zapcore.NewConsoleEncoder)) + MustRegisterEncoder("json", wrapEncoderConstructorNoError(zapcore.NewJSONEncoder)) } func newEncoder(name string, encoderConfig zapcore.EncoderConfig) (zapcore.Encoder, error) { @@ -78,7 +78,13 @@ func newEncoder(name string, encoderConfig zapcore.EncoderConfig) (zapcore.Encod } constructor, ok := encoderNameToConstructor[name] if !ok { - return nil, fmt.Errorf("no encoder registered for name %s", name) + return nil, fmt.Errorf("no encoder registered for name %q", name) + } + return constructor(encoderConfig) +} + +func wrapEncoderConstructorNoError(constructor func(zapcore.EncoderConfig) zapcore.Encoder) func(zapcore.EncoderConfig) (zapcore.Encoder, error) { + return func(encoderConfig zapcore.EncoderConfig) (zapcore.Encoder, error) { + return constructor(encoderConfig), nil } - return constructor(encoderConfig), nil } diff --git a/encoder_test.go b/encoder_test.go index f7bcf840a..65d90aa7f 100644 --- a/encoder_test.go +++ b/encoder_test.go @@ -28,12 +28,6 @@ import ( "github.com/stretchr/testify/assert" ) -var ( - newNilEncoder = func(_ zapcore.EncoderConfig) zapcore.Encoder { - return nil - } -) - func TestRegisterDefaultEncoders(t *testing.T) { testEncodersRegistered(t, "console", "json") } @@ -85,11 +79,9 @@ func TestNewEncoderNoName(t *testing.T) { } func testEncoders(f func()) { - encoderNameToConstructor = make(map[string]func(zapcore.EncoderConfig) zapcore.Encoder) - defer func() { - encoderNameToConstructor = make(map[string]func(zapcore.EncoderConfig) zapcore.Encoder) - registerDefaultEncoders() - }() + existing := encoderNameToConstructor + encoderNameToConstructor = make(map[string]func(zapcore.EncoderConfig) (zapcore.Encoder, error)) + defer func() { encoderNameToConstructor = existing }() f() } @@ -99,3 +91,7 @@ func testEncodersRegistered(t *testing.T, names ...string) { assert.NotNil(t, encoderNameToConstructor[name]) } } + +func newNilEncoder(_ zapcore.EncoderConfig) (zapcore.Encoder, error) { + return nil, nil +}