From b9f46ead20769c8e7761c73f38d2432ea4874dd4 Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Mon, 28 Aug 2017 16:52:01 +0300 Subject: [PATCH 01/15] Fix branch in coverage bage --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f902f1e0d..e9c04a4cb 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ [![Join the chat at https://gitter.im/yandex/pandora](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/yandex/pandora?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) [![Build Status](https://travis-ci.org/yandex/pandora.svg)](https://travis-ci.org/yandex/pandora) -[![Coverage Status](https://coveralls.io/repos/yandex/pandora/badge.svg?branch=master&service=github)](https://coveralls.io/github/yandex/pandora?branch=master) +[![Coverage Status](https://coveralls.io/repos/yandex/pandora/badge.svg?branch=develop&service=github)](https://coveralls.io/github/yandex/pandora?branch=develop) A load generator in Go language. From ba36b662b55cea728a9d28495968f19624052daa Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Mon, 28 Aug 2017 20:18:56 +0300 Subject: [PATCH 02/15] Restructure files in plugin pkg. --- core/plugin/doc.go | 33 ++++ core/plugin/plugin.go | 267 +++----------------------- core/plugin/plugin_suite_test.go | 70 +++++++ core/plugin/plugin_test.go | 319 ++++--------------------------- core/plugin/registry.go | 220 +++++++++++++++++++++ core/plugin/registry_test.go | 236 +++++++++++++++++++++++ 6 files changed, 619 insertions(+), 526 deletions(-) create mode 100644 core/plugin/doc.go create mode 100644 core/plugin/plugin_suite_test.go create mode 100644 core/plugin/registry.go create mode 100644 core/plugin/registry_test.go diff --git a/core/plugin/doc.go b/core/plugin/doc.go new file mode 100644 index 000000000..e99b3f863 --- /dev/null +++ b/core/plugin/doc.go @@ -0,0 +1,33 @@ +// Copyright (c) 2017 Yandex LLC. All rights reserved. +// Use of this source code is governed by a MPL 2.0 +// license that can be found in the LICENSE file. +// Author: Vladimir Skipor + +// Package plugin provides a generic inversion of control model for making +// extensible Go packages, libraries, and applications. Like +// github.com/progrium/go-extpoints, but reflect based: doesn't require code +// generation, but have more overhead; provide more flexibility, but less type +// safety. It allows to register factory for some plugin interface, and create +// new plugin instances by registered factory. +// Main feature is flexible plugin configuration: plugin factory can +// accept config struct, that could be filled by passed hook. Config default +// values could be provided by registering default config factory. +// Such flexibility can be used to decode structured text (json/yaml/etc) into +// struct with plugin interface fields. +// +// Type expectations. +// Plugin factory type should be: +// func([config ]) ([, error]) +// where configType kind is struct or struct pointer, and pluginImpl implements +// plugin interface. Plugin factory will never receive nil config, even there +// are no registered default config factory, or default config is nil. Config +// will be pointer to zero config in such case. +// If plugin factory receive config argument, default config factory can be +// registered. Default config factory type should be: is func() . +// Default config factory is optional. If no default config factory has been +// registered, than plugin factory will receive zero config (zero struct or +// pointer to zero struct). +// +// Note, that plugin interface type could be taken as reflect.TypeOf((*PluginInterface)(nil)).Elem(). +// FIXME: doc plugin factory +package plugin diff --git a/core/plugin/plugin.go b/core/plugin/plugin.go index 2882c7e0a..4ca896ba3 100644 --- a/core/plugin/plugin.go +++ b/core/plugin/plugin.go @@ -3,39 +3,10 @@ // license that can be found in the LICENSE file. // Author: Vladimir Skipor -// Package plugin provides a generic inversion of control model for making -// extensible Go packages, libraries, and applications. Like -// github.com/progrium/go-extpoints, but reflect based: doesn't require code -// generation, but have more overhead; provide more flexibility, but less type -// safety. It allows to register factory for some plugin interface, and create -// new plugin instances by registered factory. -// Main feature is flexible plugin configuration: plugin factory can -// accept config struct, that could be filled by passed hook. Config default -// values could be provided by registering default config factory. -// Such flexibility can be used to decode structured text (json/yaml/etc) into -// struct with plugin interface fields. -// -// Type expectations. -// Plugin factory type should be: -// func([config ]) ( [, error]) -// where configType kind is struct or struct pointer, and pluginImpl implements -// plugin interface. Plugin factory will never receive nil config, even there -// are no registered default config factory, or default config is nil. Config -// will be pointer to zero config in such case. -// If plugin factory receive config argument, default config factory can be -// registered. Default config factory type should be: is func() . -// Default config factory is optional. If no default config factory has been -// registered, than plugin factory will receive zero config (zero struct or -// pointer to zero struct). -// -// Note, that plugin interface type could be taken as reflect.TypeOf((*PluginInterface)(nil)).Elem(). package plugin import ( - "fmt" "reflect" - - "github.com/pkg/errors" ) // Register registers plugin factory and optional default config factory, @@ -58,8 +29,10 @@ func Lookup(pluginType reflect.Type) bool { return defaultRegistry.Lookup(pluginType) } -func LookupFactory(pluginType reflect.Type) bool { - return defaultRegistry.LookupFactory(pluginType) +// LookupFactory returns true if factoryType looks like func() (SomeInterface[, error]) +// and any plugin factory has been registered for SomeInterface. +func LookupFactory(factoryType reflect.Type) bool { + return defaultRegistry.LookupFactory(factoryType) } // New creates plugin by registered plugin factory. Returns error if creation @@ -96,226 +69,30 @@ func PtrType(ptr interface{}) reflect.Type { return t.Elem() } -func IsFactoryType(t reflect.Type) bool { - return t.Kind() == reflect.Func && - t.NumIn() == 0 && - t.NumOut() == 2 && - t.Out(0).Kind() == reflect.Interface && - t.Out(1) == errorType -} - -func FactoryPluginType(factory reflect.Type) (plugin reflect.Type, ok bool) { - if IsFactoryType(factory) { - return factory.Out(0), true - } - return -} - -type nameRegistryEntry struct { - // newPluginImpl type is func([config ]) ( [, error]), - // where configType kind is struct or struct pointer. - newPluginImpl reflect.Value - // newDefaultConfig type is func() . Zero if newPluginImpl accepts no arguments. - newDefaultConfig reflect.Value -} - -type nameRegistry map[string]nameRegistryEntry - -func newNameRegistry() nameRegistry { return make(nameRegistry) } - -type typeRegistry map[reflect.Type]nameRegistry - -var defaultRegistry = newTypeRegistry() - -func newTypeRegistry() typeRegistry { return make(typeRegistry) } - -func (r typeRegistry) Register( - pluginType reflect.Type, // plugin interface type - name string, - newPluginImpl interface{}, - newDefaultConfigOptional ...interface{}, -) { - expect(pluginType.Kind() == reflect.Interface, "plugin type should be interface, but have: %T", pluginType) - expect(name != "", "empty name") - pluginReg := r[pluginType] - if pluginReg == nil { - pluginReg = newNameRegistry() - r[pluginType] = pluginReg - } - _, ok := pluginReg[name] - expect(!ok, "plugin %s with name %q had been already registered", pluginType, name) - pluginReg[name] = newNameRegistryEntry(pluginType, newPluginImpl, newDefaultConfigOptional...) -} - -func (r typeRegistry) Lookup(pluginType reflect.Type) bool { - _, ok := r[pluginType] - return ok -} - -func (r typeRegistry) LookupFactory(factoryType reflect.Type) bool { - return IsFactoryType(factoryType) && r.Lookup(factoryType.Out(0)) -} - -func (r typeRegistry) New(pluginType reflect.Type, name string, fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { - expect(pluginType.Kind() == reflect.Interface, "plugin type should be interface, but have: %T", pluginType) - expect(name != "", "empty name") - fillConf := getFillConf(fillConfOptional) - registered, err := r.get(pluginType, name) - if err != nil { - return - } - confOptional, fillAddr := registered.NewDefaultConfig() - if fillConf != nil { - err = fillConf(fillAddr) - if err != nil { - return - } - } - return registered.NewPlugin(confOptional) -} - -func (r typeRegistry) NewFactory(factoryType reflect.Type, name string, fillConfOptional ...func(conf interface{}) error) (factory interface{}, err error) { - expect(IsFactoryType(factoryType), "plugin factory type should be like `func() (PluginInterface, error)`, but have: %T", factoryType) - expect(name != "", "empty name") - fillConf := getFillConf(fillConfOptional) - pluginType := factoryType.Out(0) - registered, err := r.get(pluginType, name) - if err != nil { - return - } - factory = reflect.MakeFunc(factoryType, func(in []reflect.Value) (out []reflect.Value) { - conf, fillAddr := registered.NewDefaultConfig() - if fillConf != nil { - // Check that config is correct. - err := fillConf(fillAddr) - if err != nil { - return []reflect.Value{reflect.Zero(pluginType), reflect.ValueOf(&err).Elem()} - } - } - out = registered.newPluginImpl.Call(conf) - if out[0].Type() != pluginType { - // Not plugin, but its implementation. - impl := out[0] - out[0] = reflect.New(pluginType).Elem() - out[0].Set(impl) - } - - if len(out) < 2 { - // Registered newPluginImpl can return no error, but we should. - out = append(out, reflect.Zero(errorType)) - } - return - }).Interface() - return -} - -func getFillConf(fillConfOptional []func(conf interface{}) error) func(interface{}) error { - expect(len(fillConfOptional) <= 1, "only fill config parameter could be passed") - if len(fillConfOptional) == 0 { - return nil - } - return fillConfOptional[0] -} - -func (e nameRegistryEntry) NewDefaultConfig() (confOptional []reflect.Value, fillAddr interface{}) { - if e.newPluginImpl.Type().NumIn() == 0 { - var emptyStruct struct{} - fillAddr = &emptyStruct // No fields to fill. - return - } - conf := e.newDefaultConfig.Call(nil)[0] - switch conf.Kind() { - case reflect.Struct: - // Config can be filled only by pointer. - if !conf.CanAddr() { - // Can't address to pass pointer into decoder. Let's make New addressable! - newArg := reflect.New(conf.Type()).Elem() - newArg.Set(conf) - conf = newArg - } - fillAddr = conf.Addr().Interface() - case reflect.Ptr: - if conf.IsNil() { - // Can't fill nil config. Init with zero. - conf = reflect.New(conf.Type().Elem()) - } - fillAddr = conf.Interface() - default: - panic("unexpected type " + conf.String()) - } - confOptional = []reflect.Value{conf} - return -} - -func (e nameRegistryEntry) NewPlugin(confOptional []reflect.Value) (plugin interface{}, err error) { - out := e.newPluginImpl.Call(confOptional) - plugin = out[0].Interface() - if len(out) > 1 { - err, _ = out[1].Interface().(error) +// FactoryPluginType returns (SomeInterface, true) if factoryType looks like func() (SomeInterface[, error]) +// or (nil, false) otherwise. +func FactoryPluginType(factoryType reflect.Type) (plugin reflect.Type, ok bool) { + if isFactoryType(factoryType) { + return factoryType.Out(0), true } return } -func newNameRegistryEntry(pluginType reflect.Type, newPluginImpl interface{}, newDefaultConfigOptional ...interface{}) nameRegistryEntry { - newPluginImplType := reflect.TypeOf(newPluginImpl) - expect(newPluginImplType.Kind() == reflect.Func, "newPluginImpl should be func") - expect(newPluginImplType.NumIn() <= 1, "newPluginImple should accept config or nothing") - expect(1 <= newPluginImplType.NumOut() && newPluginImplType.NumOut() <= 2, - "newPluginImple should return plugin implementation, and optionally error") - pluginImplType := newPluginImplType.Out(0) - expect(pluginImplType.Implements(pluginType), "pluginImpl should implement plugin interface") - if newPluginImplType.NumOut() == 2 { - expect(newPluginImplType.Out(1) == errorType, "pluginImpl should have no second return value, or it should be error") - } - - if newPluginImplType.NumIn() == 0 { - expect(len(newDefaultConfigOptional) == 0, "newPluginImpl accept no config, but newDefaultConfig passed") - return nameRegistryEntry{ - newPluginImpl: reflect.ValueOf(newPluginImpl), - } - } - - expect(len(newDefaultConfigOptional) <= 1, "only one default config newPluginImpl could be passed") - configType := newPluginImplType.In(0) - expect(configType.Kind() == reflect.Struct || - configType.Kind() == reflect.Ptr && configType.Elem().Kind() == reflect.Struct, - "unexpected config kind: %s; should be struct or struct pointer or map") - - newDefaultConfigType := reflect.FuncOf(nil, []reflect.Type{configType}, false) - var newDefaultConfig interface{} - if len(newDefaultConfigOptional) != 0 { - newDefaultConfig = newDefaultConfigOptional[0] - expect(reflect.TypeOf(newDefaultConfig) == newDefaultConfigType, - "newDefaultConfig should be func that accepst nothing, and returns newPluiginImpl argument, but have type %T", newDefaultConfig) - } else { - newDefaultConfig = reflect.MakeFunc(newDefaultConfigType, - func(_ []reflect.Value) (results []reflect.Value) { - return []reflect.Value{reflect.Zero(configType)} - }).Interface() - } - return nameRegistryEntry{ - newPluginImpl: reflect.ValueOf(newPluginImpl), - newDefaultConfig: reflect.ValueOf(newDefaultConfig), - } -} - -func (r typeRegistry) get(pluginType reflect.Type, name string) (factory nameRegistryEntry, err error) { - pluginReg, ok := r[pluginType] - if !ok { - err = errors.Errorf("no plugins for type %s has been registered", pluginType) - return +// isFactoryType returns true, if type looks like func() (SomeInterface[, error]) +func isFactoryType(t reflect.Type) bool { + hasProperParamsNum := t.Kind() == reflect.Func && + t.NumIn() == 0 && + (t.NumOut() == 1 || t.NumOut() == 2) + if !hasProperParamsNum { + return false } - factory, ok = pluginReg[name] - if !ok { - err = errors.Errorf("no plugins of type %s has been registered for name %s", pluginType, name) + if t.Out(0).Kind() != reflect.Interface { + return false } - return -} - -func expect(b bool, msg string, args ...interface{}) { - if !b { - panic(fmt.Sprintf("expectation failed: "+msg, args...)) + if t.NumOut() == 1 { + return true } + return t.Out(1) == errorType } -var errorType = reflect.TypeOf((*error)(nil)).Elem() +var defaultRegistry = newTypeRegistry() diff --git a/core/plugin/plugin_suite_test.go b/core/plugin/plugin_suite_test.go new file mode 100644 index 000000000..4d6500536 --- /dev/null +++ b/core/plugin/plugin_suite_test.go @@ -0,0 +1,70 @@ +package plugin + +import ( + "reflect" + "testing" + + "github.com/mitchellh/mapstructure" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/yandex/pandora/lib/testutil" +) + +func TestPlugin(t *testing.T) { + RegisterFailHandler(Fail) + testutil.ReplaceGlobalLogger() + RunSpecs(t, "Plugin Suite") +} + +const ( + testPluginName = "test_name" + testConfValue = "conf" + testDefaultValue = "default" + testInitValue = "init" +) + +func (r typeRegistry) testRegister(newPluginImpl interface{}, newDefaultConfigOptional ...interface{}) { + r.Register(testPluginType(), testPluginName, newPluginImpl, newDefaultConfigOptional...) +} + +func (r typeRegistry) testNew(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { + return r.New(testPluginType(), testPluginName, fillConfOptional...) +} + +func (r typeRegistry) testNewFactory(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { + factory, err := r.NewFactory(testPluginFactoryType(), testPluginName, fillConfOptional...) + if err != nil { + return + } + typedFactory := factory.(func() (testPluginInterface, error)) + return typedFactory() +} + +type testPluginInterface interface { + DoSomething() +} + +func testPluginType() reflect.Type { return reflect.TypeOf((*testPluginInterface)(nil)).Elem() } +func testPluginFactoryType() reflect.Type { + return reflect.TypeOf(func() (testPluginInterface, error) { panic("") }) +} +func newTestPlugin() *testPluginImpl { return &testPluginImpl{Value: testInitValue} } + +type testPluginImpl struct{ Value string } + +func (p *testPluginImpl) DoSomething() {} + +var _ testPluginInterface = (*testPluginImpl)(nil) + +type testPluginImplConfig struct{ Value string } + +func newTestPluginConf(c testPluginImplConfig) *testPluginImpl { return &testPluginImpl{c.Value} } +func newTestPluginDefaultConf() testPluginImplConfig { return testPluginImplConfig{testDefaultValue} } +func newTestPluginPtrConf(c *testPluginImplConfig) *testPluginImpl { + return &testPluginImpl{c.Value} +} + +func fillTestPluginConf(conf interface{}) error { + return mapstructure.Decode(map[string]interface{}{"Value": "conf"}, conf) +} diff --git a/core/plugin/plugin_test.go b/core/plugin/plugin_test.go index 27228b81f..1edbee657 100644 --- a/core/plugin/plugin_test.go +++ b/core/plugin/plugin_test.go @@ -1,286 +1,43 @@ -// Copyright (c) 2016 Yandex LLC. All rights reserved. -// Use of this source code is governed by a MPL 2.0 -// license that can be found in the LICENSE file. -// Author: Vladimir Skipor - package plugin import ( - stderrors "errors" - "fmt" - "io" - "reflect" - "testing" - - "github.com/mitchellh/mapstructure" - "github.com/pkg/errors" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" ) -func TestRegisterValid(t *testing.T) { - testCases := []struct { - description string - newPluginImpl interface{} - newDefaultConfigOptional []interface{} - }{ - {"return impl", func() *testPluginImpl { return nil }, nil}, - {"return interface", func() TestPlugin { return nil }, nil}, - {"super interface", func() interface { - io.Writer - TestPlugin - } { - return nil - }, nil}, - {"struct config", func(pluginImplConfig) TestPlugin { return nil }, nil}, - {"struct ptr config", func(*pluginImplConfig) TestPlugin { return nil }, nil}, - {"default config", func(*pluginImplConfig) TestPlugin { return nil }, []interface{}{func() *pluginImplConfig { return nil }}}, - } - for _, tc := range testCases { - t.Run(tc.description, func(t *testing.T) { - assert.NotPanics(t, func() { - newTypeRegistry().testRegister(tc.newPluginImpl, tc.newDefaultConfigOptional...) - }) - }) - } -} - -func TestRegisterInvalid(t *testing.T) { - testCases := []struct { - description string - newPluginImpl interface{} - newDefaultConfigOptional []interface{} - }{ - {"return not impl", func() testPluginImpl { panic("") }, nil}, - {"invalid config type", func(int) TestPlugin { return nil }, nil}, - {"invalid config ptr type", func(*int) TestPlugin { return nil }, nil}, - {"to many args", func(_, _ pluginImplConfig) TestPlugin { return nil }, nil}, - {"default without config", func() TestPlugin { return nil }, []interface{}{func() *pluginImplConfig { return nil }}}, - {"extra deafult config", func(*pluginImplConfig) TestPlugin { return nil }, []interface{}{func() *pluginImplConfig { return nil }, 0}}, - {"invalid default config", func(pluginImplConfig) TestPlugin { return nil }, []interface{}{func() *pluginImplConfig { return nil }}}, - {"default config accepts args", func(*pluginImplConfig) TestPlugin { return nil }, []interface{}{func(int) *pluginImplConfig { return nil }}}, - } - for _, tc := range testCases { - t.Run(tc.description, func(t *testing.T) { - defer assertExpectationFailed(t) - newTypeRegistry().testRegister(tc.newPluginImpl, tc.newDefaultConfigOptional...) - }) - } -} - -func TestRegisterNameCollisionPanics(t *testing.T) { - r := newTypeRegistry() - r.testRegister(newPlugin) - defer assertExpectationFailed(t) - r.testRegister(newPlugin) -} - -func TestLookup(t *testing.T) { - r := newTypeRegistry() - r.testRegister(newPlugin) - assert.True(t, r.Lookup(pluginType())) - assert.False(t, r.Lookup(reflect.TypeOf(0))) - assert.False(t, r.Lookup(reflect.TypeOf(&testPluginImpl{}))) - assert.False(t, r.Lookup(reflect.TypeOf((*io.Writer)(nil)).Elem())) -} - -func TestNew(t *testing.T) { - var r typeRegistry - - type New func(r typeRegistry, fillConfOptional ...func(conf interface{}) error) (interface{}, error) - var testNew New - testNewOk := func(fillConfOptional ...func(conf interface{}) error) (pluginVal string) { - plugin, err := testNew(r, fillConfOptional...) - require.NoError(t, err) - return plugin.(*testPluginImpl).Value - } - - tests := []struct { - desc string - fn func(t *testing.T) - }{ - {"no conf", func(t *testing.T) { - r.testRegister(newPlugin) - assert.Equal(t, testNewOk(), testInitValue) - }}, - {"nil error", func(t *testing.T) { - r.testRegister(func() (TestPlugin, error) { - return newPlugin(), nil - }) - assert.Equal(t, testNewOk(), testInitValue) - }}, - {"non-nil error", func(t *testing.T) { - expectedErr := stderrors.New("fill conf err") - r.testRegister(func() (TestPlugin, error) { - return nil, expectedErr - }) - _, err := testNew(r) - require.Error(t, err) - err = errors.Cause(err) - assert.Equal(t, expectedErr, err) - }}, - {"no conf, fill conf error", func(t *testing.T) { - r.testRegister(newPlugin) - expectedErr := stderrors.New("fill conf err") - _, err := testNew(r, func(_ interface{}) error { return expectedErr }) - assert.Equal(t, expectedErr, err) - }}, - {"no default", func(t *testing.T) { - r.testRegister(func(c pluginImplConfig) *testPluginImpl { return &testPluginImpl{c.Value} }) - assert.Equal(t, testNewOk(), "") - }}, - {"default", func(t *testing.T) { - r.testRegister(newPluginConf, newPluginDefaultConf) - assert.Equal(t, testNewOk(), testDefaultValue) - }}, - {"fill conf default", func(t *testing.T) { - r.testRegister(newPluginConf, newPluginDefaultConf) - assert.Equal(t, "conf", testNewOk(fillConf)) - }}, - {"fill conf no default", func(t *testing.T) { - r.testRegister(newPluginConf) - assert.Equal(t, "conf", testNewOk(fillConf)) - }}, - {"fill ptr conf no default", func(t *testing.T) { - r.testRegister(newPluginPtrConf) - assert.Equal(t, "conf", testNewOk(fillConf)) - }}, - {"no default ptr conf not nil", func(t *testing.T) { - r.testRegister(newPluginPtrConf) - assert.Equal(t, "", testNewOk()) - }}, - {"nil default, conf not nil", func(t *testing.T) { - r.testRegister(newPluginPtrConf, func() *pluginImplConfig { return nil }) - assert.Equal(t, "", testNewOk()) - }}, - {"fill nil default", func(t *testing.T) { - r.testRegister(newPluginPtrConf, func() *pluginImplConfig { return nil }) - assert.Equal(t, "conf", testNewOk(fillConf)) - }}, - {"more than one fill conf panics", func(t *testing.T) { - r.testRegister(newPluginPtrConf) - defer assertExpectationFailed(t) - testNew(r, fillConf, fillConf) - }}, - } - - for _, suite := range []struct { - new New - desc string - }{ - {typeRegistry.testNew, "New"}, - {typeRegistry.testNewFactory, "NewFactory"}, - } { - testNew = suite.new - for _, test := range tests { - r = newTypeRegistry() - t.Run(fmt.Sprintf("%s %s", suite.desc, test.desc), test.fn) - } - } - -} - -// Test typical usage. -func TestMapstructureDecode(t *testing.T) { - r := newTypeRegistry() - const nameKey = "type" - - var hook mapstructure.DecodeHookFunc - decode := func(input, result interface{}) error { - decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ - DecodeHook: hook, - ErrorUnused: true, - Result: result, - }) - if err != nil { - return err - } - return decoder.Decode(input) - } - hook = mapstructure.ComposeDecodeHookFunc( - func(from reflect.Type, to reflect.Type, data interface{}) (interface{}, error) { - if !r.Lookup(to) { - return data, nil - } - // NOTE: could be map[interface{}]interface{} here. - input := data.(map[string]interface{}) - // NOTE: should be case insensitive behaviour. - pluginName := input[nameKey].(string) - delete(input, nameKey) - return r.New(to, pluginName, func(conf interface{}) error { - // NOTE: should error, if conf has "type" field. - return decode(input, conf) - }) - }) - - r.Register(pluginType(), "my-plugin", newPluginConf, newPluginDefaultConf) - input := map[string]interface{}{ - "plugin": map[string]interface{}{ - nameKey: "my-plugin", - "value": testConfValue, - }, - } - type Config struct { - Plugin TestPlugin - } - var conf Config - err := decode(input, &conf) - require.NoError(t, err) - assert.Equal(t, testConfValue, conf.Plugin.(*testPluginImpl).Value) -} - -const ( - testPluginName = "test_name" - testConfValue = "conf" - testDefaultValue = "default" - testInitValue = "init" -) - -func (r typeRegistry) testRegister(newPluginImpl interface{}, newDefaultConfigOptional ...interface{}) { - r.Register(pluginType(), testPluginName, newPluginImpl, newDefaultConfigOptional...) -} - -func (r typeRegistry) testNew(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { - return r.New(pluginType(), testPluginName, fillConfOptional...) -} - -func (r typeRegistry) testNewFactory(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { - factory, err := r.NewFactory(pluginFactoryType(), testPluginName, fillConfOptional...) - if err != nil { - return - } - typedFactory := factory.(func() (TestPlugin, error)) - return typedFactory() -} - -type TestPlugin interface { - DoSomething() -} - -func pluginType() reflect.Type { return reflect.TypeOf((*TestPlugin)(nil)).Elem() } -func pluginFactoryType() reflect.Type { return reflect.TypeOf(func() (TestPlugin, error) { panic("") }) } -func newPlugin() *testPluginImpl { return &testPluginImpl{Value: testInitValue} } - -type testPluginImpl struct{ Value string } - -func (p *testPluginImpl) DoSomething() {} - -var _ TestPlugin = (*testPluginImpl)(nil) - -type pluginImplConfig struct{ Value string } - -func newPluginConf(c pluginImplConfig) *testPluginImpl { return &testPluginImpl{c.Value} } -func newPluginDefaultConf() pluginImplConfig { return pluginImplConfig{testDefaultValue} } -func newPluginPtrConf(c *pluginImplConfig) *testPluginImpl { - return &testPluginImpl{c.Value} -} - -func fillConf(conf interface{}) error { - return mapstructure.Decode(map[string]interface{}{"Value": "conf"}, conf) -} - -func assertExpectationFailed(t *testing.T) { - r := recover() - require.NotNil(t, r) - assert.Contains(t, r, "expectation failed") -} +var _ = Describe("Default registry", func() { + BeforeEach(func() { + Register(testPluginType(), testPluginName, newTestPlugin) + }) + AfterEach(func() { + defaultRegistry = newTypeRegistry() + }) + It("lookup", func() { + Expect(Lookup(testPluginType())).To(BeTrue()) + }) + It("lookup factory", func() { + Expect(LookupFactory(testPluginFactoryType())).To(BeTrue()) + }) + It("new", func() { + plugin, err := New(testPluginType(), testPluginName) + Expect(err).NotTo(HaveOccurred()) + Expect(plugin).NotTo(BeNil()) + }) + It("new factory", func() { + pluginFactory, err := NewFactory(testPluginFactoryType(), testPluginName) + Expect(err).NotTo(HaveOccurred()) + Expect(pluginFactory).NotTo(BeNil()) + }) +}) + +var _ = Describe("type helpers", func() { + It("ptr type", func() { + var plugin testPluginInterface + Expect(PtrType(&plugin)).To(Equal(testPluginType())) + }) + It("factory plugin type ok", func() { + factoryPlugin, ok := FactoryPluginType(testPluginFactoryType()) + Expect(ok).To(BeTrue()) + Expect(factoryPlugin).To(Equal(testPluginType())) + }) +}) diff --git a/core/plugin/registry.go b/core/plugin/registry.go new file mode 100644 index 000000000..d0bd6a56c --- /dev/null +++ b/core/plugin/registry.go @@ -0,0 +1,220 @@ +// Copyright (c) 2017 Yandex LLC. All rights reserved. +// Use of this source code is governed by a MPL 2.0 +// license that can be found in the LICENSE file. +// Author: Vladimir Skipor + +package plugin + +import ( + "fmt" + "reflect" + + "github.com/pkg/errors" +) + +type nameRegistryEntry struct { + // newPluginImpl type is func([config ]) ( [, error]), + // where configType kind is struct or struct pointer. + newPluginImpl reflect.Value + // newDefaultConfig type is func() . Zero if newPluginImpl accepts no arguments. + newDefaultConfig reflect.Value +} + +type nameRegistry map[string]nameRegistryEntry + +func newNameRegistry() nameRegistry { return make(nameRegistry) } + +type typeRegistry map[reflect.Type]nameRegistry + +func newTypeRegistry() typeRegistry { return make(typeRegistry) } + +func (r typeRegistry) Register( + pluginType reflect.Type, // plugin interface type + name string, + newPluginImpl interface{}, + newDefaultConfigOptional ...interface{}, +) { + expect(pluginType.Kind() == reflect.Interface, "plugin type should be interface, but have: %T", pluginType) + expect(name != "", "empty name") + pluginReg := r[pluginType] + if pluginReg == nil { + pluginReg = newNameRegistry() + r[pluginType] = pluginReg + } + _, ok := pluginReg[name] + expect(!ok, "plugin %s with name %q had been already registered", pluginType, name) + pluginReg[name] = newNameRegistryEntry(pluginType, newPluginImpl, newDefaultConfigOptional...) +} + +func (r typeRegistry) Lookup(pluginType reflect.Type) bool { + _, ok := r[pluginType] + return ok +} + +func (r typeRegistry) LookupFactory(factoryType reflect.Type) bool { + return isFactoryType(factoryType) && r.Lookup(factoryType.Out(0)) +} + +func (r typeRegistry) New(pluginType reflect.Type, name string, fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { + expect(pluginType.Kind() == reflect.Interface, "plugin type should be interface, but have: %T", pluginType) + expect(name != "", "empty name") + fillConf := getFillConf(fillConfOptional) + registered, err := r.get(pluginType, name) + if err != nil { + return + } + confOptional, fillAddr := registered.NewDefaultConfig() + if fillConf != nil { + err = fillConf(fillAddr) + if err != nil { + return + } + } + return registered.NewPlugin(confOptional) +} + +func (r typeRegistry) NewFactory(factoryType reflect.Type, name string, fillConfOptional ...func(conf interface{}) error) (factory interface{}, err error) { + expect(isFactoryType(factoryType), "plugin factory type should be like `func() (PluginInterface, error)`, but have: %T", factoryType) + expect(name != "", "empty name") + fillConf := getFillConf(fillConfOptional) + pluginType := factoryType.Out(0) + registered, err := r.get(pluginType, name) + if err != nil { + return + } + factory = reflect.MakeFunc(factoryType, func(in []reflect.Value) (out []reflect.Value) { + conf, fillAddr := registered.NewDefaultConfig() + if fillConf != nil { + // Check that config is correct. + err := fillConf(fillAddr) + if err != nil { + return []reflect.Value{reflect.Zero(pluginType), reflect.ValueOf(&err).Elem()} + } + } + out = registered.newPluginImpl.Call(conf) + if out[0].Type() != pluginType { + // Not plugin, but its implementation. + impl := out[0] + out[0] = reflect.New(pluginType).Elem() + out[0].Set(impl) + } + + if len(out) < 2 { + // Registered newPluginImpl can return no error, but we should. + out = append(out, reflect.Zero(errorType)) + } + return + }).Interface() + return +} + +func getFillConf(fillConfOptional []func(conf interface{}) error) func(interface{}) error { + expect(len(fillConfOptional) <= 1, "only fill config parameter could be passed") + if len(fillConfOptional) == 0 { + return nil + } + return fillConfOptional[0] +} + +func (e nameRegistryEntry) NewDefaultConfig() (confOptional []reflect.Value, fillAddr interface{}) { + if e.newPluginImpl.Type().NumIn() == 0 { + var emptyStruct struct{} + fillAddr = &emptyStruct // No fields to fill. + return + } + conf := e.newDefaultConfig.Call(nil)[0] + switch conf.Kind() { + case reflect.Struct: + // Config can be filled only by pointer. + if !conf.CanAddr() { + // Can't address to pass pointer into decoder. Let's make New addressable! + newArg := reflect.New(conf.Type()).Elem() + newArg.Set(conf) + conf = newArg + } + fillAddr = conf.Addr().Interface() + case reflect.Ptr: + if conf.IsNil() { + // Can't fill nil config. Init with zero. + conf = reflect.New(conf.Type().Elem()) + } + fillAddr = conf.Interface() + default: + panic("unexpected type " + conf.String()) + } + confOptional = []reflect.Value{conf} + return +} + +func (e nameRegistryEntry) NewPlugin(confOptional []reflect.Value) (plugin interface{}, err error) { + out := e.newPluginImpl.Call(confOptional) + plugin = out[0].Interface() + if len(out) > 1 { + err, _ = out[1].Interface().(error) + } + return +} + +func newNameRegistryEntry(pluginType reflect.Type, newPluginImpl interface{}, newDefaultConfigOptional ...interface{}) nameRegistryEntry { + newPluginImplType := reflect.TypeOf(newPluginImpl) + expect(newPluginImplType.Kind() == reflect.Func, "newPluginImpl should be func") + expect(newPluginImplType.NumIn() <= 1, "newPluginImple should accept config or nothing") + expect(1 <= newPluginImplType.NumOut() && newPluginImplType.NumOut() <= 2, + "newPluginImple should return plugin implementation, and optionally error") + pluginImplType := newPluginImplType.Out(0) + expect(pluginImplType.Implements(pluginType), "pluginImpl should implement plugin interface") + if newPluginImplType.NumOut() == 2 { + expect(newPluginImplType.Out(1) == errorType, "pluginImpl should have no second return value, or it should be error") + } + + if newPluginImplType.NumIn() == 0 { + expect(len(newDefaultConfigOptional) == 0, "newPluginImpl accept no config, but newDefaultConfig passed") + return nameRegistryEntry{ + newPluginImpl: reflect.ValueOf(newPluginImpl), + } + } + + expect(len(newDefaultConfigOptional) <= 1, "only one default config newPluginImpl could be passed") + configType := newPluginImplType.In(0) + expect(configType.Kind() == reflect.Struct || + configType.Kind() == reflect.Ptr && configType.Elem().Kind() == reflect.Struct, + "unexpected config kind: %s; should be struct or struct pointer or map") + + newDefaultConfigType := reflect.FuncOf(nil, []reflect.Type{configType}, false) + var newDefaultConfig interface{} + if len(newDefaultConfigOptional) != 0 { + newDefaultConfig = newDefaultConfigOptional[0] + expect(reflect.TypeOf(newDefaultConfig) == newDefaultConfigType, + "newDefaultConfig should be func that accepst nothing, and returns newPluiginImpl argument, but have type %T", newDefaultConfig) + } else { + newDefaultConfig = reflect.MakeFunc(newDefaultConfigType, + func(_ []reflect.Value) (results []reflect.Value) { + return []reflect.Value{reflect.Zero(configType)} + }).Interface() + } + return nameRegistryEntry{ + newPluginImpl: reflect.ValueOf(newPluginImpl), + newDefaultConfig: reflect.ValueOf(newDefaultConfig), + } +} + +func (r typeRegistry) get(pluginType reflect.Type, name string) (factory nameRegistryEntry, err error) { + pluginReg, ok := r[pluginType] + if !ok { + err = errors.Errorf("no plugins for type %s has been registered", pluginType) + return + } + factory, ok = pluginReg[name] + if !ok { + err = errors.Errorf("no plugins of type %s has been registered for name %s", pluginType, name) + } + return +} + +func expect(b bool, msg string, args ...interface{}) { + if !b { + panic(fmt.Sprintf("expectation failed: "+msg, args...)) + } +} + +var errorType = reflect.TypeOf((*error)(nil)).Elem() diff --git a/core/plugin/registry_test.go b/core/plugin/registry_test.go new file mode 100644 index 000000000..2c3293dbc --- /dev/null +++ b/core/plugin/registry_test.go @@ -0,0 +1,236 @@ +// Copyright (c) 2016 Yandex LLC. All rights reserved. +// Use of this source code is governed by a MPL 2.0 +// license that can be found in the LICENSE file. +// Author: Vladimir Skipor + +package plugin + +import ( + stderrors "errors" + "fmt" + "io" + "reflect" + "testing" + + "github.com/mitchellh/mapstructure" + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRegisterValid(t *testing.T) { + testCases := []struct { + description string + newPluginImpl interface{} + newDefaultConfigOptional []interface{} + }{ + {"return impl", func() *testPluginImpl { return nil }, nil}, + {"return interface", func() testPluginInterface { return nil }, nil}, + {"super interface", func() interface { + io.Writer + testPluginInterface + } { + return nil + }, nil}, + {"struct config", func(testPluginImplConfig) testPluginInterface { return nil }, nil}, + {"struct ptr config", func(*testPluginImplConfig) testPluginInterface { return nil }, nil}, + {"default config", func(*testPluginImplConfig) testPluginInterface { return nil }, []interface{}{func() *testPluginImplConfig { return nil }}}, + } + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + assert.NotPanics(t, func() { + newTypeRegistry().testRegister(tc.newPluginImpl, tc.newDefaultConfigOptional...) + }) + }) + } +} + +func TestRegisterInvalid(t *testing.T) { + testCases := []struct { + description string + newPluginImpl interface{} + newDefaultConfigOptional []interface{} + }{ + {"return not impl", func() testPluginImpl { panic("") }, nil}, + {"invalid config type", func(int) testPluginInterface { return nil }, nil}, + {"invalid config ptr type", func(*int) testPluginInterface { return nil }, nil}, + {"to many args", func(_, _ testPluginImplConfig) testPluginInterface { return nil }, nil}, + {"default without config", func() testPluginInterface { return nil }, []interface{}{func() *testPluginImplConfig { return nil }}}, + {"extra deafult config", func(*testPluginImplConfig) testPluginInterface { return nil }, []interface{}{func() *testPluginImplConfig { return nil }, 0}}, + {"invalid default config", func(testPluginImplConfig) testPluginInterface { return nil }, []interface{}{func() *testPluginImplConfig { return nil }}}, + {"default config accepts args", func(*testPluginImplConfig) testPluginInterface { return nil }, []interface{}{func(int) *testPluginImplConfig { return nil }}}, + } + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + defer assertExpectationFailed(t) + newTypeRegistry().testRegister(tc.newPluginImpl, tc.newDefaultConfigOptional...) + }) + } +} + +func TestRegisterNameCollisionPanics(t *testing.T) { + r := newTypeRegistry() + r.testRegister(newTestPlugin) + defer assertExpectationFailed(t) + r.testRegister(newTestPlugin) +} + +func TestLookup(t *testing.T) { + r := newTypeRegistry() + r.testRegister(newTestPlugin) + assert.True(t, r.Lookup(testPluginType())) + assert.False(t, r.Lookup(reflect.TypeOf(0))) + assert.False(t, r.Lookup(reflect.TypeOf(&testPluginImpl{}))) + assert.False(t, r.Lookup(reflect.TypeOf((*io.Writer)(nil)).Elem())) +} + +func TestNew(t *testing.T) { + var r typeRegistry + + type New func(r typeRegistry, fillConfOptional ...func(conf interface{}) error) (interface{}, error) + var testNew New + testNewOk := func(fillConfOptional ...func(conf interface{}) error) (pluginVal string) { + plugin, err := testNew(r, fillConfOptional...) + require.NoError(t, err) + return plugin.(*testPluginImpl).Value + } + + tests := []struct { + desc string + fn func(t *testing.T) + }{ + {"no conf", func(t *testing.T) { + r.testRegister(newTestPlugin) + assert.Equal(t, testNewOk(), testInitValue) + }}, + {"nil error", func(t *testing.T) { + r.testRegister(func() (testPluginInterface, error) { + return newTestPlugin(), nil + }) + assert.Equal(t, testNewOk(), testInitValue) + }}, + {"non-nil error", func(t *testing.T) { + expectedErr := stderrors.New("fill conf err") + r.testRegister(func() (testPluginInterface, error) { + return nil, expectedErr + }) + _, err := testNew(r) + require.Error(t, err) + err = errors.Cause(err) + assert.Equal(t, expectedErr, err) + }}, + {"no conf, fill conf error", func(t *testing.T) { + r.testRegister(newTestPlugin) + expectedErr := stderrors.New("fill conf err") + _, err := testNew(r, func(_ interface{}) error { return expectedErr }) + assert.Equal(t, expectedErr, err) + }}, + {"no default", func(t *testing.T) { + r.testRegister(func(c testPluginImplConfig) *testPluginImpl { return &testPluginImpl{c.Value} }) + assert.Equal(t, testNewOk(), "") + }}, + {"default", func(t *testing.T) { + r.testRegister(newTestPluginConf, newTestPluginDefaultConf) + assert.Equal(t, testNewOk(), testDefaultValue) + }}, + {"fill conf default", func(t *testing.T) { + r.testRegister(newTestPluginConf, newTestPluginDefaultConf) + assert.Equal(t, "conf", testNewOk(fillTestPluginConf)) + }}, + {"fill conf no default", func(t *testing.T) { + r.testRegister(newTestPluginConf) + assert.Equal(t, "conf", testNewOk(fillTestPluginConf)) + }}, + {"fill ptr conf no default", func(t *testing.T) { + r.testRegister(newTestPluginPtrConf) + assert.Equal(t, "conf", testNewOk(fillTestPluginConf)) + }}, + {"no default ptr conf not nil", func(t *testing.T) { + r.testRegister(newTestPluginPtrConf) + assert.Equal(t, "", testNewOk()) + }}, + {"nil default, conf not nil", func(t *testing.T) { + r.testRegister(newTestPluginPtrConf, func() *testPluginImplConfig { return nil }) + assert.Equal(t, "", testNewOk()) + }}, + {"fill nil default", func(t *testing.T) { + r.testRegister(newTestPluginPtrConf, func() *testPluginImplConfig { return nil }) + assert.Equal(t, "conf", testNewOk(fillTestPluginConf)) + }}, + {"more than one fill conf panics", func(t *testing.T) { + r.testRegister(newTestPluginPtrConf) + defer assertExpectationFailed(t) + testNew(r, fillTestPluginConf, fillTestPluginConf) + }}, + } + + for _, suite := range []struct { + new New + desc string + }{ + {typeRegistry.testNew, "New"}, + {typeRegistry.testNewFactory, "NewFactory"}, + } { + testNew = suite.new + for _, test := range tests { + r = newTypeRegistry() + t.Run(fmt.Sprintf("%s %s", suite.desc, test.desc), test.fn) + } + } + +} + +// Test typical usage. +func TestMapstructureDecode(t *testing.T) { + r := newTypeRegistry() + const nameKey = "type" + + var hook mapstructure.DecodeHookFunc + decode := func(input, result interface{}) error { + decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: hook, + ErrorUnused: true, + Result: result, + }) + if err != nil { + return err + } + return decoder.Decode(input) + } + hook = mapstructure.ComposeDecodeHookFunc( + func(from reflect.Type, to reflect.Type, data interface{}) (interface{}, error) { + if !r.Lookup(to) { + return data, nil + } + // NOTE: could be map[interface{}]interface{} here. + input := data.(map[string]interface{}) + // NOTE: should be case insensitive behaviour. + pluginName := input[nameKey].(string) + delete(input, nameKey) + return r.New(to, pluginName, func(conf interface{}) error { + // NOTE: should error, if conf has "type" field. + return decode(input, conf) + }) + }) + + r.Register(testPluginType(), "my-plugin", newTestPluginConf, newTestPluginDefaultConf) + input := map[string]interface{}{ + "plugin": map[string]interface{}{ + nameKey: "my-plugin", + "value": testConfValue, + }, + } + type Config struct { + Plugin testPluginInterface + } + var conf Config + err := decode(input, &conf) + require.NoError(t, err) + assert.Equal(t, testConfValue, conf.Plugin.(*testPluginImpl).Value) +} + +func assertExpectationFailed(t *testing.T) { + r := recover() + require.NotNil(t, r) + assert.Contains(t, r, "expectation failed") +} From 5f2a2644d290522b35de6b5b0fbe9778e42505b5 Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Sat, 2 Sep 2017 07:10:37 +0300 Subject: [PATCH 03/15] Use ginkgo and gomega in core/plugin tests --- core/plugin/registry_test.go | 373 ++++++++++++++++++----------------- 1 file changed, 188 insertions(+), 185 deletions(-) diff --git a/core/plugin/registry_test.go b/core/plugin/registry_test.go index 2c3293dbc..4930373e5 100644 --- a/core/plugin/registry_test.go +++ b/core/plugin/registry_test.go @@ -6,231 +6,234 @@ package plugin import ( - stderrors "errors" - "fmt" "io" "reflect" - "testing" "github.com/mitchellh/mapstructure" + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" "github.com/pkg/errors" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -func TestRegisterValid(t *testing.T) { - testCases := []struct { - description string - newPluginImpl interface{} - newDefaultConfigOptional []interface{} - }{ - {"return impl", func() *testPluginImpl { return nil }, nil}, - {"return interface", func() testPluginInterface { return nil }, nil}, - {"super interface", func() interface { +var _ = DescribeTable("register valid", + func( + newPluginImpl interface{}, + newDefaultConfigOptional ...interface{}, + ) { + Expect(func() { + newTypeRegistry().testRegister(newPluginImpl, newDefaultConfigOptional...) + }).NotTo(Panic()) + }, + Entry("return impl", + func() *testPluginImpl { return nil }), + Entry("return interface", + func() testPluginInterface { return nil }), + Entry("super interface", + func() interface { io.Writer testPluginInterface } { return nil - }, nil}, - {"struct config", func(testPluginImplConfig) testPluginInterface { return nil }, nil}, - {"struct ptr config", func(*testPluginImplConfig) testPluginInterface { return nil }, nil}, - {"default config", func(*testPluginImplConfig) testPluginInterface { return nil }, []interface{}{func() *testPluginImplConfig { return nil }}}, - } - for _, tc := range testCases { - t.Run(tc.description, func(t *testing.T) { - assert.NotPanics(t, func() { - newTypeRegistry().testRegister(tc.newPluginImpl, tc.newDefaultConfigOptional...) - }) - }) - } -} - -func TestRegisterInvalid(t *testing.T) { - testCases := []struct { - description string - newPluginImpl interface{} - newDefaultConfigOptional []interface{} - }{ - {"return not impl", func() testPluginImpl { panic("") }, nil}, - {"invalid config type", func(int) testPluginInterface { return nil }, nil}, - {"invalid config ptr type", func(*int) testPluginInterface { return nil }, nil}, - {"to many args", func(_, _ testPluginImplConfig) testPluginInterface { return nil }, nil}, - {"default without config", func() testPluginInterface { return nil }, []interface{}{func() *testPluginImplConfig { return nil }}}, - {"extra deafult config", func(*testPluginImplConfig) testPluginInterface { return nil }, []interface{}{func() *testPluginImplConfig { return nil }, 0}}, - {"invalid default config", func(testPluginImplConfig) testPluginInterface { return nil }, []interface{}{func() *testPluginImplConfig { return nil }}}, - {"default config accepts args", func(*testPluginImplConfig) testPluginInterface { return nil }, []interface{}{func(int) *testPluginImplConfig { return nil }}}, - } - for _, tc := range testCases { - t.Run(tc.description, func(t *testing.T) { - defer assertExpectationFailed(t) - newTypeRegistry().testRegister(tc.newPluginImpl, tc.newDefaultConfigOptional...) - }) - } -} - -func TestRegisterNameCollisionPanics(t *testing.T) { - r := newTypeRegistry() - r.testRegister(newTestPlugin) - defer assertExpectationFailed(t) - r.testRegister(newTestPlugin) -} - -func TestLookup(t *testing.T) { - r := newTypeRegistry() - r.testRegister(newTestPlugin) - assert.True(t, r.Lookup(testPluginType())) - assert.False(t, r.Lookup(reflect.TypeOf(0))) - assert.False(t, r.Lookup(reflect.TypeOf(&testPluginImpl{}))) - assert.False(t, r.Lookup(reflect.TypeOf((*io.Writer)(nil)).Elem())) -} + }), + Entry("struct config", + func(testPluginImplConfig) testPluginInterface { return nil }), + Entry("struct ptr config", + func(*testPluginImplConfig) testPluginInterface { return nil }), + Entry("default config", + func(*testPluginImplConfig) testPluginInterface { return nil }, + func() *testPluginImplConfig { return nil }), +) -func TestNew(t *testing.T) { - var r typeRegistry +var _ = DescribeTable("register invalid", + func( + newPluginImpl interface{}, + newDefaultConfigOptional ...interface{}, + ) { + Expect(func() { + defer recoverExpectationFail() + newTypeRegistry().testRegister(newPluginImpl, newDefaultConfigOptional...) + }).NotTo(Panic()) + }, + Entry("return not impl", + func() testPluginImpl { panic("") }), + Entry("invalid config type", + func(int) testPluginInterface { return nil }), + Entry("invalid config ptr type", + func(*int) testPluginInterface { return nil }), + Entry("to many args", + func(_, _ testPluginImplConfig) testPluginInterface { return nil }), + Entry("default without config", + func() testPluginInterface { return nil }, func() *testPluginImplConfig { return nil }), + Entry("extra deafult config", + func(*testPluginImplConfig) testPluginInterface { return nil }, func() *testPluginImplConfig { return nil }, 0), + Entry("invalid default config", + func(testPluginImplConfig) testPluginInterface { return nil }, func() *testPluginImplConfig { return nil }), + Entry("default config accepts args", + func(*testPluginImplConfig) testPluginInterface { return nil }, func(int) *testPluginImplConfig { return nil }), +) +var _ = Describe("registry", func() { + It("register name collision panics", func() { + r := newTypeRegistry() + r.testRegister(newTestPlugin) + defer recoverExpectationFail() + r.testRegister(newTestPlugin) + }) + It("lookup", func() { + r := newTypeRegistry() + r.testRegister(newTestPlugin) + Expect(r.Lookup(testPluginType())).To(BeTrue()) + Expect(r.Lookup(reflect.TypeOf(0))).To(BeFalse()) + Expect(r.Lookup(reflect.TypeOf(&testPluginImpl{}))).To(BeFalse()) + Expect(r.Lookup(reflect.TypeOf((*io.Writer)(nil)).Elem())).To(BeFalse()) + }) + +}) + +var _ = Describe("new", func() { type New func(r typeRegistry, fillConfOptional ...func(conf interface{}) error) (interface{}, error) - var testNew New - testNewOk := func(fillConfOptional ...func(conf interface{}) error) (pluginVal string) { - plugin, err := testNew(r, fillConfOptional...) - require.NoError(t, err) - return plugin.(*testPluginImpl).Value - } - - tests := []struct { - desc string - fn func(t *testing.T) - }{ - {"no conf", func(t *testing.T) { + var ( + r typeRegistry + testNew New + testNewOk = func(fillConfOptional ...func(conf interface{}) error) (pluginVal string) { + plugin, err := testNew(r, fillConfOptional...) + Expect(err).NotTo(HaveOccurred()) + return plugin.(*testPluginImpl).Value + } + ) + BeforeEach(func() { r = newTypeRegistry() }) + runTestCases := func() { + It("no conf", func() { r.testRegister(newTestPlugin) - assert.Equal(t, testNewOk(), testInitValue) - }}, - {"nil error", func(t *testing.T) { + Expect(testNewOk()).To(Equal(testInitValue)) + }) + It("nil error", func() { r.testRegister(func() (testPluginInterface, error) { return newTestPlugin(), nil }) - assert.Equal(t, testNewOk(), testInitValue) - }}, - {"non-nil error", func(t *testing.T) { - expectedErr := stderrors.New("fill conf err") + Expect(testNewOk()).To(Equal(testInitValue)) + }) + It("non-nil error", func() { + expectedErr := errors.New("fill conf err") r.testRegister(func() (testPluginInterface, error) { return nil, expectedErr }) _, err := testNew(r) - require.Error(t, err) + Expect(err).To(HaveOccurred()) err = errors.Cause(err) - assert.Equal(t, expectedErr, err) - }}, - {"no conf, fill conf error", func(t *testing.T) { + Expect(expectedErr).To(Equal(err)) + }) + It("no conf, fill conf error", func() { r.testRegister(newTestPlugin) - expectedErr := stderrors.New("fill conf err") + expectedErr := errors.New("fill conf err") _, err := testNew(r, func(_ interface{}) error { return expectedErr }) - assert.Equal(t, expectedErr, err) - }}, - {"no default", func(t *testing.T) { + Expect(expectedErr).To(Equal(err)) + }) + It("no default", func() { r.testRegister(func(c testPluginImplConfig) *testPluginImpl { return &testPluginImpl{c.Value} }) - assert.Equal(t, testNewOk(), "") - }}, - {"default", func(t *testing.T) { + Expect(testNewOk()).To(Equal("")) + }) + It("default", func() { r.testRegister(newTestPluginConf, newTestPluginDefaultConf) - assert.Equal(t, testNewOk(), testDefaultValue) - }}, - {"fill conf default", func(t *testing.T) { + Expect(testNewOk()).To(Equal(testDefaultValue)) + }) + It("fill conf default", func() { r.testRegister(newTestPluginConf, newTestPluginDefaultConf) - assert.Equal(t, "conf", testNewOk(fillTestPluginConf)) - }}, - {"fill conf no default", func(t *testing.T) { + Expect("conf").To(Equal(testNewOk(fillTestPluginConf))) + }) + It("fill conf no default", func() { r.testRegister(newTestPluginConf) - assert.Equal(t, "conf", testNewOk(fillTestPluginConf)) - }}, - {"fill ptr conf no default", func(t *testing.T) { + Expect("conf").To(Equal(testNewOk(fillTestPluginConf))) + }) + It("fill ptr conf no default", func() { r.testRegister(newTestPluginPtrConf) - assert.Equal(t, "conf", testNewOk(fillTestPluginConf)) - }}, - {"no default ptr conf not nil", func(t *testing.T) { + Expect("conf").To(Equal(testNewOk(fillTestPluginConf))) + }) + It("no default ptr conf not nil", func() { r.testRegister(newTestPluginPtrConf) - assert.Equal(t, "", testNewOk()) - }}, - {"nil default, conf not nil", func(t *testing.T) { + Expect("").To(Equal(testNewOk())) + }) + It("nil default, conf not nil", func() { r.testRegister(newTestPluginPtrConf, func() *testPluginImplConfig { return nil }) - assert.Equal(t, "", testNewOk()) - }}, - {"fill nil default", func(t *testing.T) { + Expect("").To(Equal(testNewOk())) + }) + It("fill nil default", func() { r.testRegister(newTestPluginPtrConf, func() *testPluginImplConfig { return nil }) - assert.Equal(t, "conf", testNewOk(fillTestPluginConf)) - }}, - {"more than one fill conf panics", func(t *testing.T) { + Expect("conf").To(Equal(testNewOk(fillTestPluginConf))) + }) + It("more than one fill conf panics", func() { r.testRegister(newTestPluginPtrConf) - defer assertExpectationFailed(t) + defer recoverExpectationFail() testNew(r, fillTestPluginConf, fillTestPluginConf) - }}, - } - - for _, suite := range []struct { - new New - desc string - }{ - {typeRegistry.testNew, "New"}, - {typeRegistry.testNewFactory, "NewFactory"}, - } { - testNew = suite.new - for _, test := range tests { - r = newTypeRegistry() - t.Run(fmt.Sprintf("%s %s", suite.desc, test.desc), test.fn) - } - } - -} - -// Test typical usage. -func TestMapstructureDecode(t *testing.T) { - r := newTypeRegistry() - const nameKey = "type" - - var hook mapstructure.DecodeHookFunc - decode := func(input, result interface{}) error { - decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ - DecodeHook: hook, - ErrorUnused: true, - Result: result, }) - if err != nil { - return err - } - return decoder.Decode(input) } - hook = mapstructure.ComposeDecodeHookFunc( - func(from reflect.Type, to reflect.Type, data interface{}) (interface{}, error) { - if !r.Lookup(to) { - return data, nil + Context("use New", func() { + BeforeEach(func() { testNew = typeRegistry.testNew }) + runTestCases() + + }) + Context("use NewFactory", func() { + BeforeEach(func() { testNew = typeRegistry.testNewFactory }) + runTestCases() + }) + +}) + +var _ = Describe("decode", func() { + It("ok", func() { + r := newTypeRegistry() + const nameKey = "type" + + var hook mapstructure.DecodeHookFunc + decode := func(input, result interface{}) error { + decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: hook, + ErrorUnused: true, + Result: result, + }) + if err != nil { + return err } - // NOTE: could be map[interface{}]interface{} here. - input := data.(map[string]interface{}) - // NOTE: should be case insensitive behaviour. - pluginName := input[nameKey].(string) - delete(input, nameKey) - return r.New(to, pluginName, func(conf interface{}) error { - // NOTE: should error, if conf has "type" field. - return decode(input, conf) + return decoder.Decode(input) + } + hook = mapstructure.ComposeDecodeHookFunc( + func(from reflect.Type, to reflect.Type, data interface{}) (interface{}, error) { + if !r.Lookup(to) { + return data, nil + } + // NOTE: could be map[interface{}]interface{} here. + input := data.(map[string]interface{}) + // NOTE: should be case insensitive behaviour. + pluginName := input[nameKey].(string) + delete(input, nameKey) + return r.New(to, pluginName, func(conf interface{}) error { + // NOTE: should error, if conf has "type" field. + return decode(input, conf) + }) }) - }) - r.Register(testPluginType(), "my-plugin", newTestPluginConf, newTestPluginDefaultConf) - input := map[string]interface{}{ - "plugin": map[string]interface{}{ - nameKey: "my-plugin", - "value": testConfValue, - }, - } - type Config struct { - Plugin testPluginInterface - } - var conf Config - err := decode(input, &conf) - require.NoError(t, err) - assert.Equal(t, testConfValue, conf.Plugin.(*testPluginImpl).Value) -} + r.Register(testPluginType(), "my-plugin", newTestPluginConf, newTestPluginDefaultConf) + input := map[string]interface{}{ + "plugin": map[string]interface{}{ + nameKey: "my-plugin", + "value": testConfValue, + }, + } + type Config struct { + Plugin testPluginInterface + } + var conf Config + err := decode(input, &conf) + Expect(err).NotTo(HaveOccurred()) + actualValue := conf.Plugin.(*testPluginImpl).Value + Expect(actualValue).To(Equal(testConfValue)) + }) + +}) -func assertExpectationFailed(t *testing.T) { +func recoverExpectationFail() { r := recover() - require.NotNil(t, r) - assert.Contains(t, r, "expectation failed") + Expect(r).NotTo(BeNil()) + Expect(r).To(ContainSubstring("expectation failed")) } From c2dbbbfe126b279031be84030ea65beb854d6869 Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Sat, 2 Sep 2017 07:44:33 +0300 Subject: [PATCH 04/15] Extract default config logic. --- core/plugin/registry.go | 80 ++++++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 37 deletions(-) diff --git a/core/plugin/registry.go b/core/plugin/registry.go index d0bd6a56c..af14c53c8 100644 --- a/core/plugin/registry.go +++ b/core/plugin/registry.go @@ -12,12 +12,48 @@ import ( "github.com/pkg/errors" ) +type newDefaultConfigContainer struct { + // value type is func() . !IsValid() if newPluginImpl accepts no arguments. + value reflect.Value +} + +// In reflect pkg []Value used to call functions. It's easier to return it, that convert from pointer when needed. +func (e newDefaultConfigContainer) Get() (maybeConf []reflect.Value, fillAddr interface{}) { + configRequired := e.value.IsValid() + if !configRequired { + var emptyStruct struct{} + fillAddr = &emptyStruct // No fields to fill. + return + } + conf := e.value.Call(nil)[0] + switch conf.Kind() { + case reflect.Struct: + // Config can be filled only by pointer. + if !conf.CanAddr() { + // Can't address to pass pointer into decoder. Let's make New addressable! + newArg := reflect.New(conf.Type()).Elem() + newArg.Set(conf) + conf = newArg + } + fillAddr = conf.Addr().Interface() + case reflect.Ptr: + if conf.IsNil() { + // Can't fill nil config. Init with zero. + conf = reflect.New(conf.Type().Elem()) + } + fillAddr = conf.Interface() + default: + panic("unexpected type " + conf.String()) + } + maybeConf = []reflect.Value{conf} + return +} + type nameRegistryEntry struct { // newPluginImpl type is func([config ]) ( [, error]), // where configType kind is struct or struct pointer. newPluginImpl reflect.Value - // newDefaultConfig type is func() . Zero if newPluginImpl accepts no arguments. - newDefaultConfig reflect.Value + defaultConfig newDefaultConfigContainer } type nameRegistry map[string]nameRegistryEntry @@ -63,14 +99,14 @@ func (r typeRegistry) New(pluginType reflect.Type, name string, fillConfOptional if err != nil { return } - confOptional, fillAddr := registered.NewDefaultConfig() + conf, fillAddr := registered.defaultConfig.Get() if fillConf != nil { err = fillConf(fillAddr) if err != nil { return } } - return registered.NewPlugin(confOptional) + return registered.NewPlugin(conf) } func (r typeRegistry) NewFactory(factoryType reflect.Type, name string, fillConfOptional ...func(conf interface{}) error) (factory interface{}, err error) { @@ -83,7 +119,7 @@ func (r typeRegistry) NewFactory(factoryType reflect.Type, name string, fillConf return } factory = reflect.MakeFunc(factoryType, func(in []reflect.Value) (out []reflect.Value) { - conf, fillAddr := registered.NewDefaultConfig() + conf, fillAddr := registered.defaultConfig.Get() if fillConf != nil { // Check that config is correct. err := fillConf(fillAddr) @@ -116,36 +152,6 @@ func getFillConf(fillConfOptional []func(conf interface{}) error) func(interface return fillConfOptional[0] } -func (e nameRegistryEntry) NewDefaultConfig() (confOptional []reflect.Value, fillAddr interface{}) { - if e.newPluginImpl.Type().NumIn() == 0 { - var emptyStruct struct{} - fillAddr = &emptyStruct // No fields to fill. - return - } - conf := e.newDefaultConfig.Call(nil)[0] - switch conf.Kind() { - case reflect.Struct: - // Config can be filled only by pointer. - if !conf.CanAddr() { - // Can't address to pass pointer into decoder. Let's make New addressable! - newArg := reflect.New(conf.Type()).Elem() - newArg.Set(conf) - conf = newArg - } - fillAddr = conf.Addr().Interface() - case reflect.Ptr: - if conf.IsNil() { - // Can't fill nil config. Init with zero. - conf = reflect.New(conf.Type().Elem()) - } - fillAddr = conf.Interface() - default: - panic("unexpected type " + conf.String()) - } - confOptional = []reflect.Value{conf} - return -} - func (e nameRegistryEntry) NewPlugin(confOptional []reflect.Value) (plugin interface{}, err error) { out := e.newPluginImpl.Call(confOptional) plugin = out[0].Interface() @@ -193,8 +199,8 @@ func newNameRegistryEntry(pluginType reflect.Type, newPluginImpl interface{}, ne }).Interface() } return nameRegistryEntry{ - newPluginImpl: reflect.ValueOf(newPluginImpl), - newDefaultConfig: reflect.ValueOf(newDefaultConfig), + newPluginImpl: reflect.ValueOf(newPluginImpl), + defaultConfig: newDefaultConfigContainer{reflect.ValueOf(newDefaultConfig)}, } } From 05c04a8b09101a62fad70de139556871bc5dfc27 Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Sat, 30 Sep 2017 16:38:11 +0300 Subject: [PATCH 05/15] Extract plugin constructor logic. --- core/plugin/constructor.go | 72 +++++++++ core/plugin/constructor_test.go | 64 ++++++++ core/plugin/plugin.go | 9 ++ core/plugin/plugin_suite_test.go | 50 +++++-- core/plugin/plugin_test.go | 4 +- core/plugin/registry.go | 247 ++++++++++++++----------------- core/plugin/registry_test.go | 117 ++++++++++----- 7 files changed, 379 insertions(+), 184 deletions(-) create mode 100644 core/plugin/constructor.go create mode 100644 core/plugin/constructor_test.go diff --git a/core/plugin/constructor.go b/core/plugin/constructor.go new file mode 100644 index 000000000..411fe3f9a --- /dev/null +++ b/core/plugin/constructor.go @@ -0,0 +1,72 @@ +// Copyright (c) 2017 Yandex LLC. All rights reserved. +// Use of this source code is governed by a MPL 2.0 +// license that can be found in the LICENSE file. +// Author: Vladimir Skipor + +package plugin + +import "reflect" + +type constructor interface { + NewPlugin(maybeConf []reflect.Value) (plugin interface{}, err error) + NewFactory(factoryType reflect.Type, getMaybeConf func() ([]reflect.Value, error)) (pluginFactory interface{}, err error) +} + +func newPluginConstructor(pluginType reflect.Type, newPlugin interface{}) *pluginConstructor { + newPluginType := reflect.TypeOf(newPlugin) + expect(newPluginType.Kind() == reflect.Func, "plugin constructor should be func") + expect(newPluginType.NumIn() <= 1, "plugin constructor should accept config or nothing") + expect(1 <= newPluginType.NumOut() && newPluginType.NumOut() <= 2, + "plugin constructor should return plugin implementation, and optionally error") + pluginImplType := newPluginType.Out(0) + expect(pluginImplType.Implements(pluginType), "plugin constructor should implement plugin interface") + if newPluginType.NumOut() == 2 { + expect(newPluginType.Out(1) == errorType, "plugin constructor should have no second return value, or it should be error") + } + newPluginVal := reflect.ValueOf(newPlugin) + return &pluginConstructor{pluginType, newPluginVal} +} + +type pluginConstructor struct { + pluginType reflect.Type + // newPlugin type is func([config ]) ( [, error]), + // where configType kind is struct or struct pointer. + newPlugin reflect.Value +} + +func (c *pluginConstructor) NewPlugin(maybeConf []reflect.Value) (plugin interface{}, err error) { + out := c.newPlugin.Call(maybeConf) + plugin = out[0].Interface() + if len(out) > 1 { + err, _ = out[1].Interface().(error) + } + return +} + +func (c *pluginConstructor) NewFactory(factoryType reflect.Type, getMaybeConf func() ([]reflect.Value, error)) (interface{}, error) { + // FIXME: if no config and same type return newPlugin interface + // FIXME: support no error returning factories + return reflect.MakeFunc(factoryType, func(in []reflect.Value) (out []reflect.Value) { + var conf []reflect.Value + if getMaybeConf != nil { + var err error + conf, err = getMaybeConf() + if err != nil { + return []reflect.Value{reflect.Zero(c.pluginType), reflect.ValueOf(&err).Elem()} + } + } + out = c.newPlugin.Call(conf) + if out[0].Type() != c.pluginType { + // Not plugin, but its implementation. + impl := out[0] + out[0] = reflect.New(c.pluginType).Elem() + out[0].Set(impl) + } + + if len(out) < 2 { + // Registered newPlugin can return no error, but we should. + out = append(out, reflect.Zero(errorType)) + } + return + }).Interface(), nil +} diff --git a/core/plugin/constructor_test.go b/core/plugin/constructor_test.go new file mode 100644 index 000000000..ab836e43e --- /dev/null +++ b/core/plugin/constructor_test.go @@ -0,0 +1,64 @@ +// Copyright (c) 2017 Yandex LLC. All rights reserved. +// Use of this source code is governed by a MPL 2.0 +// license that can be found in the LICENSE file. +// Author: Vladimir Skipor + +package plugin + +import ( + "reflect" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + "github.com/pkg/errors" +) + +var _ = Describe("plugin constructor", func() { + DescribeTable("expectations failed", + func(pluginType reflect.Type, newPlugin interface{}) { + defer recoverExpectationFail() + newPluginConstructor(pluginType, newPlugin) + Expect(true) + }, + Entry("not func", + errorType, errors.New("that is not constructor")), + Entry("not implements", + errorType, newTestPluginImpl), + Entry("too many args", + testPluginType(), func(_, _ testPluginConfig) testPlugin { panic("") }), + Entry("too many return valued", + testPluginType(), func() (_ testPlugin, _, _ error) { panic("") }), + Entry("second return value is not error", + testPluginType(), func() (_, _ testPlugin) { panic("") }), + ) + confToMaybe := func(conf interface{}) []reflect.Value { + if conf != nil { + return []reflect.Value{reflect.ValueOf(conf)} + } + return nil + } + var ( + conf interface{} + // Auto set. + //getMaybeConf = func() []reflect.Value { + // return confToMaybe(conf) + //} + ) + BeforeEach(func() { conf = nil }) + + It("new plugin", func() { + testee := newPluginConstructor(testPluginType(), newTestPlugin) + plugin, err := testee.NewPlugin(nil) + Expect(err).NotTo(HaveOccurred()) + expectConfigValue(plugin, testInitValue) + }) + + It("new config plugin ", func() { + testee := newPluginConstructor(testPluginType(), newTestPluginImplConf) + plugin, err := testee.NewPlugin(confToMaybe(newTestPluginDefaultConf())) + Expect(err).NotTo(HaveOccurred()) + expectConfigValue(plugin, testDefaultValue) + }) + +}) diff --git a/core/plugin/plugin.go b/core/plugin/plugin.go index 4ca896ba3..0fe491fad 100644 --- a/core/plugin/plugin.go +++ b/core/plugin/plugin.go @@ -6,6 +6,7 @@ package plugin import ( + "fmt" "reflect" ) @@ -96,3 +97,11 @@ func isFactoryType(t reflect.Type) bool { } var defaultRegistry = newTypeRegistry() + +var errorType = reflect.TypeOf((*error)(nil)).Elem() + +func expect(b bool, msg string, args ...interface{}) { + if !b { + panic(fmt.Sprintf("expectation failed: "+msg, args...)) + } +} diff --git a/core/plugin/plugin_suite_test.go b/core/plugin/plugin_suite_test.go index 4d6500536..f82926f15 100644 --- a/core/plugin/plugin_suite_test.go +++ b/core/plugin/plugin_suite_test.go @@ -4,10 +4,10 @@ import ( "reflect" "testing" - "github.com/mitchellh/mapstructure" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/yandex/pandora/core/config" "github.com/yandex/pandora/lib/testutil" ) @@ -19,9 +19,9 @@ func TestPlugin(t *testing.T) { const ( testPluginName = "test_name" - testConfValue = "conf" testDefaultValue = "default" testInitValue = "init" + testFilledValue = "conf" ) func (r typeRegistry) testRegister(newPluginImpl interface{}, newDefaultConfigOptional ...interface{}) { @@ -37,34 +37,58 @@ func (r typeRegistry) testNewFactory(fillConfOptional ...func(conf interface{}) if err != nil { return } - typedFactory := factory.(func() (testPluginInterface, error)) + typedFactory := factory.(func() (testPlugin, error)) return typedFactory() } -type testPluginInterface interface { +type testPlugin interface { DoSomething() } -func testPluginType() reflect.Type { return reflect.TypeOf((*testPluginInterface)(nil)).Elem() } +func testPluginType() reflect.Type { return reflect.TypeOf((*testPlugin)(nil)).Elem() } +func testPluginImplType() reflect.Type { return reflect.TypeOf((*testPluginImpl)(nil)).Elem() } func testPluginFactoryType() reflect.Type { - return reflect.TypeOf(func() (testPluginInterface, error) { panic("") }) + return reflect.TypeOf(func() (testPlugin, error) { panic("") }) +} +func testPluginImplFactoryType() reflect.Type { + return reflect.TypeOf(func() (*testPluginImpl, error) { panic("") }) +} +func testPluginNoErrFactoryType() reflect.Type { + return reflect.TypeOf(func() testPlugin { panic("") }) } -func newTestPlugin() *testPluginImpl { return &testPluginImpl{Value: testInitValue} } type testPluginImpl struct{ Value string } func (p *testPluginImpl) DoSomething() {} -var _ testPluginInterface = (*testPluginImpl)(nil) +var _ testPlugin = (*testPluginImpl)(nil) -type testPluginImplConfig struct{ Value string } +type testPluginConfig struct{ Value string } -func newTestPluginConf(c testPluginImplConfig) *testPluginImpl { return &testPluginImpl{c.Value} } -func newTestPluginDefaultConf() testPluginImplConfig { return testPluginImplConfig{testDefaultValue} } -func newTestPluginPtrConf(c *testPluginImplConfig) *testPluginImpl { +func newTestPlugin() testPlugin { return newTestPluginImpl() } +func newTestPluginImpl() *testPluginImpl { return &testPluginImpl{Value: testInitValue} } +func newTestPluginImplConf(c testPluginConfig) *testPluginImpl { return &testPluginImpl{c.Value} } +func newTestPluginImplPtrConf(c *testPluginConfig) *testPluginImpl { return &testPluginImpl{c.Value} } +func newTestPluginDefaultConf() testPluginConfig { return testPluginConfig{testDefaultValue} } +func newTestPluginDefaultPtrConf() *testPluginConfig { return &testPluginConfig{testDefaultValue} } + func fillTestPluginConf(conf interface{}) error { - return mapstructure.Decode(map[string]interface{}{"Value": "conf"}, conf) + return config.Decode(map[string]interface{}{"Value": testFilledValue}, conf) +} + +func expectConfigValue(conf interface{}, val string) { + conf.(confChecker).expectValue(val) } + +type confChecker interface { + expectValue(string) +} + +var _ confChecker = testPluginConfig{} +var _ confChecker = &testPluginImpl{} + +func (c testPluginConfig) expectValue(val string) { Expect(c.Value).To(Equal(val)) } +func (p *testPluginImpl) expectValue(val string) { Expect(p.Value).To(Equal(val)) } diff --git a/core/plugin/plugin_test.go b/core/plugin/plugin_test.go index 1edbee657..d262f87a9 100644 --- a/core/plugin/plugin_test.go +++ b/core/plugin/plugin_test.go @@ -7,7 +7,7 @@ import ( var _ = Describe("Default registry", func() { BeforeEach(func() { - Register(testPluginType(), testPluginName, newTestPlugin) + Register(testPluginType(), testPluginName, newTestPluginImpl) }) AfterEach(func() { defaultRegistry = newTypeRegistry() @@ -32,7 +32,7 @@ var _ = Describe("Default registry", func() { var _ = Describe("type helpers", func() { It("ptr type", func() { - var plugin testPluginInterface + var plugin testPlugin Expect(PtrType(&plugin)).To(Equal(testPluginType())) }) It("factory plugin type ok", func() { diff --git a/core/plugin/registry.go b/core/plugin/registry.go index af14c53c8..fb29671bb 100644 --- a/core/plugin/registry.go +++ b/core/plugin/registry.go @@ -6,63 +6,25 @@ package plugin import ( - "fmt" "reflect" "github.com/pkg/errors" ) -type newDefaultConfigContainer struct { - // value type is func() . !IsValid() if newPluginImpl accepts no arguments. - value reflect.Value -} - -// In reflect pkg []Value used to call functions. It's easier to return it, that convert from pointer when needed. -func (e newDefaultConfigContainer) Get() (maybeConf []reflect.Value, fillAddr interface{}) { - configRequired := e.value.IsValid() - if !configRequired { - var emptyStruct struct{} - fillAddr = &emptyStruct // No fields to fill. - return - } - conf := e.value.Call(nil)[0] - switch conf.Kind() { - case reflect.Struct: - // Config can be filled only by pointer. - if !conf.CanAddr() { - // Can't address to pass pointer into decoder. Let's make New addressable! - newArg := reflect.New(conf.Type()).Elem() - newArg.Set(conf) - conf = newArg - } - fillAddr = conf.Addr().Interface() - case reflect.Ptr: - if conf.IsNil() { - // Can't fill nil config. Init with zero. - conf = reflect.New(conf.Type().Elem()) - } - fillAddr = conf.Interface() - default: - panic("unexpected type " + conf.String()) - } - maybeConf = []reflect.Value{conf} - return -} - -type nameRegistryEntry struct { - // newPluginImpl type is func([config ]) ( [, error]), - // where configType kind is struct or struct pointer. - newPluginImpl reflect.Value - defaultConfig newDefaultConfigContainer -} +// FIXME: make public. +func newTypeRegistry() typeRegistry { return make(typeRegistry) } -type nameRegistry map[string]nameRegistryEntry +// FIXME: encapsucate to struct and make public +type typeRegistry map[reflect.Type]nameRegistry func newNameRegistry() nameRegistry { return make(nameRegistry) } -type typeRegistry map[reflect.Type]nameRegistry +type nameRegistry map[string]nameRegistryEntry -func newTypeRegistry() typeRegistry { return make(typeRegistry) } +type nameRegistryEntry struct { + constructor constructor + defaultConfig defaultConfigContainer +} func (r typeRegistry) Register( pluginType reflect.Type, // plugin interface type @@ -79,7 +41,8 @@ func (r typeRegistry) Register( } _, ok := pluginReg[name] expect(!ok, "plugin %s with name %q had been already registered", pluginType, name) - pluginReg[name] = newNameRegistryEntry(pluginType, newPluginImpl, newDefaultConfigOptional...) + newDefaultConfig := getNewDefaultConfig(newDefaultConfigOptional) + pluginReg[name] = newNameRegistryEntry(pluginType, newPluginImpl, newDefaultConfig) } func (r typeRegistry) Lookup(pluginType reflect.Type) bool { @@ -99,14 +62,11 @@ func (r typeRegistry) New(pluginType reflect.Type, name string, fillConfOptional if err != nil { return } - conf, fillAddr := registered.defaultConfig.Get() - if fillConf != nil { - err = fillConf(fillAddr) - if err != nil { - return - } + conf, err := registered.defaultConfig.Get(fillConf) + if err != nil { + return nil, err } - return registered.NewPlugin(conf) + return registered.constructor.NewPlugin(conf) } func (r typeRegistry) NewFactory(factoryType reflect.Type, name string, fillConfOptional ...func(conf interface{}) error) (factory interface{}, err error) { @@ -118,109 +78,128 @@ func (r typeRegistry) NewFactory(factoryType reflect.Type, name string, fillConf if err != nil { return } - factory = reflect.MakeFunc(factoryType, func(in []reflect.Value) (out []reflect.Value) { - conf, fillAddr := registered.defaultConfig.Get() - if fillConf != nil { - // Check that config is correct. - err := fillConf(fillAddr) - if err != nil { - return []reflect.Value{reflect.Zero(pluginType), reflect.ValueOf(&err).Elem()} - } - } - out = registered.newPluginImpl.Call(conf) - if out[0].Type() != pluginType { - // Not plugin, but its implementation. - impl := out[0] - out[0] = reflect.New(pluginType).Elem() - out[0].Set(impl) - } - - if len(out) < 2 { - // Registered newPluginImpl can return no error, but we should. - out = append(out, reflect.Zero(errorType)) - } - return - }).Interface() - return + // OPTIMIZE: not create getMaybeConfig when there no config required. Just check it once on empty struct ptr. + getMaybeConfig := func() ([]reflect.Value, error) { + return registered.defaultConfig.Get(fillConf) + } + return registered.constructor.NewFactory(factoryType, getMaybeConfig) } -func getFillConf(fillConfOptional []func(conf interface{}) error) func(interface{}) error { - expect(len(fillConfOptional) <= 1, "only fill config parameter could be passed") - if len(fillConfOptional) == 0 { - return nil - } - return fillConfOptional[0] +func newNameRegistryEntry(pluginType reflect.Type, newPluginImpl interface{}, newDefaultConfig interface{}) nameRegistryEntry { + constructor := newPluginConstructor(pluginType, newPluginImpl) + defaultConfig := newDefaultConfigContainer(reflect.TypeOf(newPluginImpl), newDefaultConfig) + return nameRegistryEntry{constructor, defaultConfig} } -func (e nameRegistryEntry) NewPlugin(confOptional []reflect.Value) (plugin interface{}, err error) { - out := e.newPluginImpl.Call(confOptional) - plugin = out[0].Interface() - if len(out) > 1 { - err, _ = out[1].Interface().(error) +func (r typeRegistry) get(pluginType reflect.Type, name string) (factory nameRegistryEntry, err error) { + pluginReg, ok := r[pluginType] + if !ok { + err = errors.Errorf("no plugins for type %s has been registered", pluginType) + return + } + factory, ok = pluginReg[name] + if !ok { + err = errors.Errorf("no plugins of type %s has been registered for name %s", pluginType, name) } return } -func newNameRegistryEntry(pluginType reflect.Type, newPluginImpl interface{}, newDefaultConfigOptional ...interface{}) nameRegistryEntry { - newPluginImplType := reflect.TypeOf(newPluginImpl) - expect(newPluginImplType.Kind() == reflect.Func, "newPluginImpl should be func") - expect(newPluginImplType.NumIn() <= 1, "newPluginImple should accept config or nothing") - expect(1 <= newPluginImplType.NumOut() && newPluginImplType.NumOut() <= 2, - "newPluginImple should return plugin implementation, and optionally error") - pluginImplType := newPluginImplType.Out(0) - expect(pluginImplType.Implements(pluginType), "pluginImpl should implement plugin interface") - if newPluginImplType.NumOut() == 2 { - expect(newPluginImplType.Out(1) == errorType, "pluginImpl should have no second return value, or it should be error") - } +// defaultConfigContainer contains default config creation logic. +// Zero value is valid and means that no config is needed. +type defaultConfigContainer struct { + // !IsValid() if constructor accepts no arguments. + // Otherwise type is func() . + newValue reflect.Value +} - if newPluginImplType.NumIn() == 0 { - expect(len(newDefaultConfigOptional) == 0, "newPluginImpl accept no config, but newDefaultConfig passed") - return nameRegistryEntry{ - newPluginImpl: reflect.ValueOf(newPluginImpl), - } +// newDefaultConfigContainer should not be called +func newDefaultConfigContainer(constructorType reflect.Type, newDefaultConfig interface{}) defaultConfigContainer { + if constructorType.NumIn() == 0 { + expect(newDefaultConfig == nil, "constructor accept no config, but newDefaultConfig passed") + return defaultConfigContainer{} } - - expect(len(newDefaultConfigOptional) <= 1, "only one default config newPluginImpl could be passed") - configType := newPluginImplType.In(0) + expect(constructorType.NumIn() == 1, "constructor should accept zero or one argument") + configType := constructorType.In(0) expect(configType.Kind() == reflect.Struct || configType.Kind() == reflect.Ptr && configType.Elem().Kind() == reflect.Struct, - "unexpected config kind: %s; should be struct or struct pointer or map") - + "unexpected config kind: %s; should be struct or struct pointer") newDefaultConfigType := reflect.FuncOf(nil, []reflect.Type{configType}, false) - var newDefaultConfig interface{} - if len(newDefaultConfigOptional) != 0 { - newDefaultConfig = newDefaultConfigOptional[0] - expect(reflect.TypeOf(newDefaultConfig) == newDefaultConfigType, - "newDefaultConfig should be func that accepst nothing, and returns newPluiginImpl argument, but have type %T", newDefaultConfig) - } else { - newDefaultConfig = reflect.MakeFunc(newDefaultConfigType, + if newDefaultConfig == nil { + value := reflect.MakeFunc(newDefaultConfigType, func(_ []reflect.Value) (results []reflect.Value) { + // OPTIMIZE: create addressable. return []reflect.Value{reflect.Zero(configType)} - }).Interface() + }) + return defaultConfigContainer{value} } - return nameRegistryEntry{ - newPluginImpl: reflect.ValueOf(newPluginImpl), - defaultConfig: newDefaultConfigContainer{reflect.ValueOf(newDefaultConfig)}, + value := reflect.ValueOf(newDefaultConfig) + expect(value.Type() == newDefaultConfigType, + "newDefaultConfig should be func that accepts nothing, and returns constructor argument, but have type %T", newDefaultConfig) + return defaultConfigContainer{value} +} + +// In reflect pkg []Value used to call functions. It's easier to return it, that convert from pointer when needed. +func (e defaultConfigContainer) Get(fillConf func(fillAddr interface{}) error) (maybeConf []reflect.Value, err error) { + var fillAddr interface{} + if e.configRequired() { + maybeConf, fillAddr = e.new() + } else { + var emptyStruct struct{} + fillAddr = &emptyStruct // No fields to fill. + } + if fillConf != nil { + err = fillConf(fillAddr) + if err != nil { + return nil, err + } } + return } -func (r typeRegistry) get(pluginType reflect.Type, name string) (factory nameRegistryEntry, err error) { - pluginReg, ok := r[pluginType] - if !ok { - err = errors.Errorf("no plugins for type %s has been registered", pluginType) - return +func (e defaultConfigContainer) new() (maybeConf []reflect.Value, fillAddr interface{}) { + if !e.configRequired() { + panic("try to create config when not required") } - factory, ok = pluginReg[name] - if !ok { - err = errors.Errorf("no plugins of type %s has been registered for name %s", pluginType, name) + conf := e.newValue.Call(nil)[0] + switch conf.Kind() { + case reflect.Struct: + // Config can be filled only by pointer. + if !conf.CanAddr() { + // Can't address to pass pointer into decoder. Let's make New addressable! + newArg := reflect.New(conf.Type()).Elem() + newArg.Set(conf) + conf = newArg + } + fillAddr = conf.Addr().Interface() + case reflect.Ptr: + if conf.IsNil() { + // Can't fill nil config. Init with zero. + conf = reflect.New(conf.Type().Elem()) + } + fillAddr = conf.Interface() + default: + panic("unexpected type " + conf.String()) } + maybeConf = []reflect.Value{conf} return } -func expect(b bool, msg string, args ...interface{}) { - if !b { - panic(fmt.Sprintf("expectation failed: "+msg, args...)) +func (e defaultConfigContainer) configRequired() bool { + return e.newValue.IsValid() +} + +func getFillConf(fillConfOptional []func(conf interface{}) error) func(interface{}) error { + expect(len(fillConfOptional) <= 1, "only fill config parameter could be passed") + if len(fillConfOptional) == 0 { + return nil } + return fillConfOptional[0] } -var errorType = reflect.TypeOf((*error)(nil)).Elem() +func getNewDefaultConfig(newDefaultConfigOptional []interface{}) interface{} { + expect(len(newDefaultConfigOptional) <= 1, "too many arguments passed") + if len(newDefaultConfigOptional) == 0 { + return nil + } + return newDefaultConfigOptional[0] +} diff --git a/core/plugin/registry_test.go b/core/plugin/registry_test.go index 4930373e5..b52bf209c 100644 --- a/core/plugin/registry_test.go +++ b/core/plugin/registry_test.go @@ -16,6 +16,53 @@ import ( "github.com/pkg/errors" ) +var _ = Describe("new default config container", func() { + DescribeTable("expectation fail", + func(constructor interface{}, newDefaultConfigOptional ...interface{}) { + newDefaultConfig := getNewDefaultConfig(newDefaultConfigOptional) + defer recoverExpectationFail() + newDefaultConfigContainer(reflect.TypeOf(constructor), newDefaultConfig) + }, + Entry("invalid type", + func(int) testPlugin { return nil }), + Entry("invalid ptr type", + func(*int) testPlugin { return nil }), + Entry("to many args", + func(_, _ testPluginConfig) testPlugin { return nil }), + Entry("default without config", + func() testPlugin { return nil }, func() *testPluginConfig { return nil }), + Entry("invalid default config", + func(testPluginConfig) testPlugin { return nil }, func() *testPluginConfig { return nil }), + Entry("default config accepts args", + func(*testPluginConfig) testPlugin { return nil }, func(int) *testPluginConfig { return nil }), + ) + + DescribeTable("expectation ok", + func(constructor interface{}, newDefaultConfigOptional ...interface{}) { + newDefaultConfig := getNewDefaultConfig(newDefaultConfigOptional) + container := newDefaultConfigContainer(reflect.TypeOf(constructor), newDefaultConfig) + conf, err := container.Get(fillTestPluginConf) + Expect(err).NotTo(HaveOccurred()) + Expect(conf).To(HaveLen(1)) + expectConfigValue(conf[0].Interface(), testFilledValue) + }, + Entry("no default config", + newTestPluginImplConf), + Entry("no default ptr config", + newTestPluginImplPtrConf), + Entry("default config", + newTestPluginImplConf, newTestPluginDefaultConf), + Entry("default ptr config", + newTestPluginImplPtrConf, newTestPluginDefaultPtrConf), + ) + + It("fill no config failed", func() { + container := newDefaultConfigContainer(testPluginFactoryType(), nil) + _, err := container.Get(fillTestPluginConf) + Expect(err).To(HaveOccurred()) + }) +}) + var _ = DescribeTable("register valid", func( newPluginImpl interface{}, @@ -28,21 +75,21 @@ var _ = DescribeTable("register valid", Entry("return impl", func() *testPluginImpl { return nil }), Entry("return interface", - func() testPluginInterface { return nil }), + func() testPlugin { return nil }), Entry("super interface", func() interface { io.Writer - testPluginInterface + testPlugin } { return nil }), Entry("struct config", - func(testPluginImplConfig) testPluginInterface { return nil }), + func(testPluginConfig) testPlugin { return nil }), Entry("struct ptr config", - func(*testPluginImplConfig) testPluginInterface { return nil }), + func(*testPluginConfig) testPlugin { return nil }), Entry("default config", - func(*testPluginImplConfig) testPluginInterface { return nil }, - func() *testPluginImplConfig { return nil }), + func(*testPluginConfig) testPlugin { return nil }, + func() *testPluginConfig { return nil }), ) var _ = DescribeTable("register invalid", @@ -58,31 +105,31 @@ var _ = DescribeTable("register invalid", Entry("return not impl", func() testPluginImpl { panic("") }), Entry("invalid config type", - func(int) testPluginInterface { return nil }), + func(int) testPlugin { return nil }), Entry("invalid config ptr type", - func(*int) testPluginInterface { return nil }), + func(*int) testPlugin { return nil }), Entry("to many args", - func(_, _ testPluginImplConfig) testPluginInterface { return nil }), + func(_, _ testPluginConfig) testPlugin { return nil }), Entry("default without config", - func() testPluginInterface { return nil }, func() *testPluginImplConfig { return nil }), - Entry("extra deafult config", - func(*testPluginImplConfig) testPluginInterface { return nil }, func() *testPluginImplConfig { return nil }, 0), + func() testPlugin { return nil }, func() *testPluginConfig { return nil }), + Entry("extra default config", + func(*testPluginConfig) testPlugin { return nil }, func() *testPluginConfig { return nil }, 0), Entry("invalid default config", - func(testPluginImplConfig) testPluginInterface { return nil }, func() *testPluginImplConfig { return nil }), + func(testPluginConfig) testPlugin { return nil }, func() *testPluginConfig { return nil }), Entry("default config accepts args", - func(*testPluginImplConfig) testPluginInterface { return nil }, func(int) *testPluginImplConfig { return nil }), + func(*testPluginConfig) testPlugin { return nil }, func(int) *testPluginConfig { return nil }), ) var _ = Describe("registry", func() { It("register name collision panics", func() { r := newTypeRegistry() - r.testRegister(newTestPlugin) + r.testRegister(newTestPluginImpl) defer recoverExpectationFail() - r.testRegister(newTestPlugin) + r.testRegister(newTestPluginImpl) }) It("lookup", func() { r := newTypeRegistry() - r.testRegister(newTestPlugin) + r.testRegister(newTestPluginImpl) Expect(r.Lookup(testPluginType())).To(BeTrue()) Expect(r.Lookup(reflect.TypeOf(0))).To(BeFalse()) Expect(r.Lookup(reflect.TypeOf(&testPluginImpl{}))).To(BeFalse()) @@ -105,18 +152,18 @@ var _ = Describe("new", func() { BeforeEach(func() { r = newTypeRegistry() }) runTestCases := func() { It("no conf", func() { - r.testRegister(newTestPlugin) + r.testRegister(newTestPluginImpl) Expect(testNewOk()).To(Equal(testInitValue)) }) It("nil error", func() { - r.testRegister(func() (testPluginInterface, error) { - return newTestPlugin(), nil + r.testRegister(func() (testPlugin, error) { + return newTestPluginImpl(), nil }) Expect(testNewOk()).To(Equal(testInitValue)) }) It("non-nil error", func() { expectedErr := errors.New("fill conf err") - r.testRegister(func() (testPluginInterface, error) { + r.testRegister(func() (testPlugin, error) { return nil, expectedErr }) _, err := testNew(r) @@ -125,45 +172,45 @@ var _ = Describe("new", func() { Expect(expectedErr).To(Equal(err)) }) It("no conf, fill conf error", func() { - r.testRegister(newTestPlugin) + r.testRegister(newTestPluginImpl) expectedErr := errors.New("fill conf err") _, err := testNew(r, func(_ interface{}) error { return expectedErr }) Expect(expectedErr).To(Equal(err)) }) It("no default", func() { - r.testRegister(func(c testPluginImplConfig) *testPluginImpl { return &testPluginImpl{c.Value} }) + r.testRegister(func(c testPluginConfig) *testPluginImpl { return &testPluginImpl{c.Value} }) Expect(testNewOk()).To(Equal("")) }) It("default", func() { - r.testRegister(newTestPluginConf, newTestPluginDefaultConf) + r.testRegister(newTestPluginImplConf, newTestPluginDefaultConf) Expect(testNewOk()).To(Equal(testDefaultValue)) }) It("fill conf default", func() { - r.testRegister(newTestPluginConf, newTestPluginDefaultConf) + r.testRegister(newTestPluginImplConf, newTestPluginDefaultConf) Expect("conf").To(Equal(testNewOk(fillTestPluginConf))) }) It("fill conf no default", func() { - r.testRegister(newTestPluginConf) + r.testRegister(newTestPluginImplConf) Expect("conf").To(Equal(testNewOk(fillTestPluginConf))) }) It("fill ptr conf no default", func() { - r.testRegister(newTestPluginPtrConf) + r.testRegister(newTestPluginImplPtrConf) Expect("conf").To(Equal(testNewOk(fillTestPluginConf))) }) It("no default ptr conf not nil", func() { - r.testRegister(newTestPluginPtrConf) + r.testRegister(newTestPluginImplPtrConf) Expect("").To(Equal(testNewOk())) }) It("nil default, conf not nil", func() { - r.testRegister(newTestPluginPtrConf, func() *testPluginImplConfig { return nil }) + r.testRegister(newTestPluginImplPtrConf, func() *testPluginConfig { return nil }) Expect("").To(Equal(testNewOk())) }) It("fill nil default", func() { - r.testRegister(newTestPluginPtrConf, func() *testPluginImplConfig { return nil }) + r.testRegister(newTestPluginImplPtrConf, func() *testPluginConfig { return nil }) Expect("conf").To(Equal(testNewOk(fillTestPluginConf))) }) It("more than one fill conf panics", func() { - r.testRegister(newTestPluginPtrConf) + r.testRegister(newTestPluginImplPtrConf) defer recoverExpectationFail() testNew(r, fillTestPluginConf, fillTestPluginConf) }) @@ -213,21 +260,21 @@ var _ = Describe("decode", func() { }) }) - r.Register(testPluginType(), "my-plugin", newTestPluginConf, newTestPluginDefaultConf) + r.Register(testPluginType(), "my-plugin", newTestPluginImplConf, newTestPluginDefaultConf) input := map[string]interface{}{ "plugin": map[string]interface{}{ nameKey: "my-plugin", - "value": testConfValue, + "value": testFilledValue, }, } type Config struct { - Plugin testPluginInterface + Plugin testPlugin } var conf Config err := decode(input, &conf) Expect(err).NotTo(HaveOccurred()) actualValue := conf.Plugin.(*testPluginImpl).Value - Expect(actualValue).To(Equal(testConfValue)) + Expect(actualValue).To(Equal(testFilledValue)) }) }) From 6a0396dce27b684a8811c03a0f2fa519b2429483 Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Sat, 30 Sep 2017 16:52:27 +0300 Subject: [PATCH 06/15] Export plugin.Registry type --- core/plugin/plugin.go | 12 ++++++---- core/plugin/plugin_suite_test.go | 6 ++--- core/plugin/plugin_test.go | 2 +- core/plugin/registry.go | 38 +++++++++++++++++--------------- core/plugin/registry_test.go | 20 ++++++++--------- 5 files changed, 42 insertions(+), 36 deletions(-) diff --git a/core/plugin/plugin.go b/core/plugin/plugin.go index 0fe491fad..24082cd5a 100644 --- a/core/plugin/plugin.go +++ b/core/plugin/plugin.go @@ -10,6 +10,10 @@ import ( "reflect" ) +func DefaultRegistry() *Registry { + return defaultRegistry +} + // Register registers plugin factory and optional default config factory, // for given plugin interface type and plugin name. // See package doc for type expectations details. @@ -21,19 +25,19 @@ func Register( newPluginImpl interface{}, newDefaultConfigOptional ...interface{}, ) { - defaultRegistry.Register(pluginType, name, newPluginImpl, newDefaultConfigOptional...) + DefaultRegistry().Register(pluginType, name, newPluginImpl, newDefaultConfigOptional...) } // Lookup returns true if any plugin factory has been registered for given // type. func Lookup(pluginType reflect.Type) bool { - return defaultRegistry.Lookup(pluginType) + return DefaultRegistry().Lookup(pluginType) } // LookupFactory returns true if factoryType looks like func() (SomeInterface[, error]) // and any plugin factory has been registered for SomeInterface. func LookupFactory(factoryType reflect.Type) bool { - return defaultRegistry.LookupFactory(factoryType) + return DefaultRegistry().LookupFactory(factoryType) } // New creates plugin by registered plugin factory. Returns error if creation @@ -96,7 +100,7 @@ func isFactoryType(t reflect.Type) bool { return t.Out(1) == errorType } -var defaultRegistry = newTypeRegistry() +var defaultRegistry = NewRegistry() var errorType = reflect.TypeOf((*error)(nil)).Elem() diff --git a/core/plugin/plugin_suite_test.go b/core/plugin/plugin_suite_test.go index f82926f15..2487080c5 100644 --- a/core/plugin/plugin_suite_test.go +++ b/core/plugin/plugin_suite_test.go @@ -24,15 +24,15 @@ const ( testFilledValue = "conf" ) -func (r typeRegistry) testRegister(newPluginImpl interface{}, newDefaultConfigOptional ...interface{}) { +func (r *Registry) testRegister(newPluginImpl interface{}, newDefaultConfigOptional ...interface{}) { r.Register(testPluginType(), testPluginName, newPluginImpl, newDefaultConfigOptional...) } -func (r typeRegistry) testNew(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { +func (r *Registry) testNew(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { return r.New(testPluginType(), testPluginName, fillConfOptional...) } -func (r typeRegistry) testNewFactory(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { +func (r *Registry) testNewFactory(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { factory, err := r.NewFactory(testPluginFactoryType(), testPluginName, fillConfOptional...) if err != nil { return diff --git a/core/plugin/plugin_test.go b/core/plugin/plugin_test.go index d262f87a9..7179b5199 100644 --- a/core/plugin/plugin_test.go +++ b/core/plugin/plugin_test.go @@ -10,7 +10,7 @@ var _ = Describe("Default registry", func() { Register(testPluginType(), testPluginName, newTestPluginImpl) }) AfterEach(func() { - defaultRegistry = newTypeRegistry() + defaultRegistry = NewRegistry() }) It("lookup", func() { Expect(Lookup(testPluginType())).To(BeTrue()) diff --git a/core/plugin/registry.go b/core/plugin/registry.go index fb29671bb..5cff28490 100644 --- a/core/plugin/registry.go +++ b/core/plugin/registry.go @@ -11,11 +11,13 @@ import ( "github.com/pkg/errors" ) -// FIXME: make public. -func newTypeRegistry() typeRegistry { return make(typeRegistry) } +func NewRegistry() *Registry { + return &Registry{make(map[reflect.Type]nameRegistry)} +} -// FIXME: encapsucate to struct and make public -type typeRegistry map[reflect.Type]nameRegistry +type Registry struct { + typeToNameReg map[reflect.Type]nameRegistry +} func newNameRegistry() nameRegistry { return make(nameRegistry) } @@ -26,7 +28,7 @@ type nameRegistryEntry struct { defaultConfig defaultConfigContainer } -func (r typeRegistry) Register( +func (r *Registry) Register( pluginType reflect.Type, // plugin interface type name string, newPluginImpl interface{}, @@ -34,27 +36,27 @@ func (r typeRegistry) Register( ) { expect(pluginType.Kind() == reflect.Interface, "plugin type should be interface, but have: %T", pluginType) expect(name != "", "empty name") - pluginReg := r[pluginType] - if pluginReg == nil { - pluginReg = newNameRegistry() - r[pluginType] = pluginReg + nameReg := r.typeToNameReg[pluginType] + if nameReg == nil { + nameReg = newNameRegistry() + r.typeToNameReg[pluginType] = nameReg } - _, ok := pluginReg[name] + _, ok := nameReg[name] expect(!ok, "plugin %s with name %q had been already registered", pluginType, name) newDefaultConfig := getNewDefaultConfig(newDefaultConfigOptional) - pluginReg[name] = newNameRegistryEntry(pluginType, newPluginImpl, newDefaultConfig) + nameReg[name] = newNameRegistryEntry(pluginType, newPluginImpl, newDefaultConfig) } -func (r typeRegistry) Lookup(pluginType reflect.Type) bool { - _, ok := r[pluginType] +func (r *Registry) Lookup(pluginType reflect.Type) bool { + _, ok := r.typeToNameReg[pluginType] return ok } -func (r typeRegistry) LookupFactory(factoryType reflect.Type) bool { +func (r *Registry) LookupFactory(factoryType reflect.Type) bool { return isFactoryType(factoryType) && r.Lookup(factoryType.Out(0)) } -func (r typeRegistry) New(pluginType reflect.Type, name string, fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { +func (r *Registry) New(pluginType reflect.Type, name string, fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { expect(pluginType.Kind() == reflect.Interface, "plugin type should be interface, but have: %T", pluginType) expect(name != "", "empty name") fillConf := getFillConf(fillConfOptional) @@ -69,7 +71,7 @@ func (r typeRegistry) New(pluginType reflect.Type, name string, fillConfOptional return registered.constructor.NewPlugin(conf) } -func (r typeRegistry) NewFactory(factoryType reflect.Type, name string, fillConfOptional ...func(conf interface{}) error) (factory interface{}, err error) { +func (r *Registry) NewFactory(factoryType reflect.Type, name string, fillConfOptional ...func(conf interface{}) error) (factory interface{}, err error) { expect(isFactoryType(factoryType), "plugin factory type should be like `func() (PluginInterface, error)`, but have: %T", factoryType) expect(name != "", "empty name") fillConf := getFillConf(fillConfOptional) @@ -91,8 +93,8 @@ func newNameRegistryEntry(pluginType reflect.Type, newPluginImpl interface{}, ne return nameRegistryEntry{constructor, defaultConfig} } -func (r typeRegistry) get(pluginType reflect.Type, name string) (factory nameRegistryEntry, err error) { - pluginReg, ok := r[pluginType] +func (r *Registry) get(pluginType reflect.Type, name string) (factory nameRegistryEntry, err error) { + pluginReg, ok := r.typeToNameReg[pluginType] if !ok { err = errors.Errorf("no plugins for type %s has been registered", pluginType) return diff --git a/core/plugin/registry_test.go b/core/plugin/registry_test.go index b52bf209c..f8e7d050e 100644 --- a/core/plugin/registry_test.go +++ b/core/plugin/registry_test.go @@ -69,7 +69,7 @@ var _ = DescribeTable("register valid", newDefaultConfigOptional ...interface{}, ) { Expect(func() { - newTypeRegistry().testRegister(newPluginImpl, newDefaultConfigOptional...) + NewRegistry().testRegister(newPluginImpl, newDefaultConfigOptional...) }).NotTo(Panic()) }, Entry("return impl", @@ -99,7 +99,7 @@ var _ = DescribeTable("register invalid", ) { Expect(func() { defer recoverExpectationFail() - newTypeRegistry().testRegister(newPluginImpl, newDefaultConfigOptional...) + NewRegistry().testRegister(newPluginImpl, newDefaultConfigOptional...) }).NotTo(Panic()) }, Entry("return not impl", @@ -122,13 +122,13 @@ var _ = DescribeTable("register invalid", var _ = Describe("registry", func() { It("register name collision panics", func() { - r := newTypeRegistry() + r := NewRegistry() r.testRegister(newTestPluginImpl) defer recoverExpectationFail() r.testRegister(newTestPluginImpl) }) It("lookup", func() { - r := newTypeRegistry() + r := NewRegistry() r.testRegister(newTestPluginImpl) Expect(r.Lookup(testPluginType())).To(BeTrue()) Expect(r.Lookup(reflect.TypeOf(0))).To(BeFalse()) @@ -139,9 +139,9 @@ var _ = Describe("registry", func() { }) var _ = Describe("new", func() { - type New func(r typeRegistry, fillConfOptional ...func(conf interface{}) error) (interface{}, error) + type New func(r *Registry, fillConfOptional ...func(conf interface{}) error) (interface{}, error) var ( - r typeRegistry + r *Registry testNew New testNewOk = func(fillConfOptional ...func(conf interface{}) error) (pluginVal string) { plugin, err := testNew(r, fillConfOptional...) @@ -149,7 +149,7 @@ var _ = Describe("new", func() { return plugin.(*testPluginImpl).Value } ) - BeforeEach(func() { r = newTypeRegistry() }) + BeforeEach(func() { r = NewRegistry() }) runTestCases := func() { It("no conf", func() { r.testRegister(newTestPluginImpl) @@ -216,12 +216,12 @@ var _ = Describe("new", func() { }) } Context("use New", func() { - BeforeEach(func() { testNew = typeRegistry.testNew }) + BeforeEach(func() { testNew = (*Registry).testNew }) runTestCases() }) Context("use NewFactory", func() { - BeforeEach(func() { testNew = typeRegistry.testNewFactory }) + BeforeEach(func() { testNew = (*Registry).testNewFactory }) runTestCases() }) @@ -229,7 +229,7 @@ var _ = Describe("new", func() { var _ = Describe("decode", func() { It("ok", func() { - r := newTypeRegistry() + r := NewRegistry() const nameKey = "type" var hook mapstructure.DecodeHookFunc From 6bfb3cb56697b11d50019bd5133a9146ca221876 Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Sat, 30 Sep 2017 19:21:23 +0300 Subject: [PATCH 07/15] Support no err returning factories in constructors --- core/plugin/constructor.go | 129 +++++++++++++++++++++++---- core/plugin/constructor_test.go | 126 ++++++++++++++++++++++++-- core/plugin/internal/ptest/plugin.go | 6 ++ core/plugin/plugin_suite_test.go | 28 ++++-- core/plugin/plugin_test.go | 5 ++ core/plugin/registry_test.go | 10 +-- 6 files changed, 267 insertions(+), 37 deletions(-) create mode 100644 core/plugin/internal/ptest/plugin.go diff --git a/core/plugin/constructor.go b/core/plugin/constructor.go index 411fe3f9a..0f95b6c1f 100644 --- a/core/plugin/constructor.go +++ b/core/plugin/constructor.go @@ -5,10 +5,22 @@ package plugin -import "reflect" +import ( + "fmt" + "reflect" +) +// constructor interface representing ability to create some plugin interface +// implementations and is't factory functions. Usually it wraps some newPlugin or newFactory function, and +// use is as implementation creator. +// constructor expects, that caller pass correct maybeConf value, that can be +// passed to underlying implementation creator. type constructor interface { + // NewPlugin constructs plugin implementation. NewPlugin(maybeConf []reflect.Value) (plugin interface{}, err error) + // getMaybeConf may be nil, if no config required. + // Underlying implementation creator may require new config for every instance create. + // If so, then getMaybeConf will be called on every instance create. Otherwise, only once. NewFactory(factoryType reflect.Type, getMaybeConf func() ([]reflect.Value, error)) (pluginFactory interface{}, err error) } @@ -27,6 +39,8 @@ func newPluginConstructor(pluginType reflect.Type, newPlugin interface{}) *plugi return &pluginConstructor{pluginType, newPluginVal} } +// pluginConstructor is abstract constructor of some pluginType interface implementations +// and it's factory functions, using some func newPlugin func. type pluginConstructor struct { pluginType reflect.Type // newPlugin type is func([config ]) ( [, error]), @@ -44,29 +58,108 @@ func (c *pluginConstructor) NewPlugin(maybeConf []reflect.Value) (plugin interfa } func (c *pluginConstructor) NewFactory(factoryType reflect.Type, getMaybeConf func() ([]reflect.Value, error)) (interface{}, error) { - // FIXME: if no config and same type return newPlugin interface - // FIXME: support no error returning factories - return reflect.MakeFunc(factoryType, func(in []reflect.Value) (out []reflect.Value) { - var conf []reflect.Value + // FIXME: TEST no config and same type return newPlugin interface + // FIXME: TEST no error returning factories + if c.newPlugin.Type() == factoryType { + return c.newPlugin.Interface(), nil + } + return reflect.MakeFunc(factoryType, func(in []reflect.Value) []reflect.Value { + var maybeConf []reflect.Value if getMaybeConf != nil { var err error - conf, err = getMaybeConf() + maybeConf, err = getMaybeConf() if err != nil { - return []reflect.Value{reflect.Zero(c.pluginType), reflect.ValueOf(&err).Elem()} + switch factoryType.NumOut() { + case 1: + panic(err) + case 2: + return []reflect.Value{reflect.Zero(c.pluginType), reflect.ValueOf(&err).Elem()} + default: + panic(fmt.Sprintf(" out params num expeced to be 1 or 2, but have: %v", factoryType.NumOut())) + } } } - out = c.newPlugin.Call(conf) - if out[0].Type() != c.pluginType { - // Not plugin, but its implementation. - impl := out[0] - out[0] = reflect.New(c.pluginType).Elem() - out[0].Set(impl) - } + out := c.newPlugin.Call(maybeConf) + return convertFactoryOutParam(c.pluginType, factoryType.NumOut(), out) + }).Interface(), nil +} + +// factoryConstructor is abstract constructor of some pluginType interface and it's +// factory functions, using some func newFactory func. +type factoryConstructor struct { + pluginType reflect.Type + // newFactory type is func([config ]) (func() ( [, error])), + // where configType kind is struct or struct pointer. + newFactory reflect.Value +} - if len(out) < 2 { - // Registered newPlugin can return no error, but we should. - out = append(out, reflect.Zero(errorType)) +func (c *factoryConstructor) NewPlugin(maybeConf []reflect.Value) (plugin interface{}, err error) { + factory, err := c.callNewFactory(maybeConf) + if err != nil { + return nil, err + } + out := factory.Call(nil) + plugin = out[0].Interface() + if len(out) > 1 { + err, _ = out[1].Interface().(error) + } + return +} + +func (c *factoryConstructor) NewFactory(factoryType reflect.Type, getMaybeConf func() ([]reflect.Value, error)) (interface{}, error) { + var maybeConf []reflect.Value + if getMaybeConf != nil { + var err error + maybeConf, err = getMaybeConf() + if err != nil { + return nil, err } - return + } + factory, err := c.callNewFactory(maybeConf) + if err != nil { + return nil, err + } + if factory.Type() == factoryType { + return factory.Interface(), nil + } + return reflect.MakeFunc(factoryType, func(in []reflect.Value) []reflect.Value { + out := factory.Call(nil) + return convertFactoryOutParam(c.pluginType, factoryType.NumOut(), out) }).Interface(), nil } + +func (c *factoryConstructor) callNewFactory(maybeConf []reflect.Value) (factory reflect.Value, err error) { + factoryAndMaybeErr := c.newFactory.Call(maybeConf) + if len(factoryAndMaybeErr) > 1 { + err, _ = factoryAndMaybeErr[1].Interface().(error) + } + return factoryAndMaybeErr[0], err +} + +// convertFactoryOutParam converts output params of some factory (newFactory) call, to required. +func convertFactoryOutParam(pluginType reflect.Type, numOut int, out []reflect.Value) []reflect.Value { + switch numOut { + case 1, 2: + // OK. + default: + panic(fmt.Sprintf("unexpeced out params num: %v; 1 or 2 expected", numOut)) + } + if out[0].Type() != pluginType { + // Not plugin, but its implementation. + impl := out[0] + out[0] = reflect.New(pluginType).Elem() + out[0].Set(impl) + } + if len(out) < numOut { + // Registered factory returns no error, but we should. + out = append(out, reflect.Zero(errorType)) + } + if numOut < len(out) { + // Registered factory returns error, but we should not. + if !out[1].IsNil() { + panic(out[1].Interface()) + } + out = out[:1] + } + return out +} diff --git a/core/plugin/constructor_test.go b/core/plugin/constructor_test.go index ab836e43e..ccdf07bb7 100644 --- a/core/plugin/constructor_test.go +++ b/core/plugin/constructor_test.go @@ -8,6 +8,8 @@ package plugin import ( "reflect" + "fmt" + . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" @@ -38,14 +40,18 @@ var _ = Describe("plugin constructor", func() { } return nil } - var ( - conf interface{} - // Auto set. - //getMaybeConf = func() []reflect.Value { - // return confToMaybe(conf) - //} - ) - BeforeEach(func() { conf = nil }) + + confToGetMaybe := func(conf interface{}) func() ([]reflect.Value, error) { + return func() ([]reflect.Value, error) { + return confToMaybe(conf), nil + } + } + + errToGetMaybe := func(err error) func() ([]reflect.Value, error) { + return func() ([]reflect.Value, error) { + return nil, err + } + } It("new plugin", func() { testee := newPluginConstructor(testPluginType(), newTestPlugin) @@ -56,9 +62,111 @@ var _ = Describe("plugin constructor", func() { It("new config plugin ", func() { testee := newPluginConstructor(testPluginType(), newTestPluginImplConf) - plugin, err := testee.NewPlugin(confToMaybe(newTestPluginDefaultConf())) + plugin, err := testee.NewPlugin(confToMaybe(newTestDefaultConf())) + Expect(err).NotTo(HaveOccurred()) + expectConfigValue(plugin, testDefaultValue) + }) + + It("new plugin failed", func() { + testee := newPluginConstructor(testPluginType(), newTestPluginImplErrFailed) + plugin, err := testee.NewPlugin(nil) + Expect(err).To(Equal(testPluginCreateFailedErr)) + Expect(plugin).To(BeNil()) + }) + + It("new factory from conversion", func() { + testee := newPluginConstructor(testPluginType(), newTestPlugin) + factory, err := testee.NewFactory(testPluginNoErrFactoryType(), nil) + Expect(err).NotTo(HaveOccurred()) + expectSameFunc(factory, newTestPlugin) + }) + + It("new factory from new impl", func() { + testee := newPluginConstructor(testPluginType(), newTestPluginImpl) + factory, err := testee.NewFactory(testPluginNoErrFactoryType(), nil) + Expect(err).NotTo(HaveOccurred()) + f, ok := factory.(func() testPlugin) + Expect(ok).To(BeTrue()) + plugin := f() + expectConfigValue(plugin, testInitValue) + }) + + It("new factory add err", func() { + testee := newPluginConstructor(testPluginType(), newTestPlugin) + factory, err := testee.NewFactory(testPluginFactoryType(), nil) + Expect(err).NotTo(HaveOccurred()) + f, ok := factory.(func() (testPlugin, error)) + Expect(ok).To(BeTrue()) + plugin, err := f() + Expect(err).NotTo(HaveOccurred()) + expectConfigValue(plugin, testInitValue) + }) + + It("new factory trim nil err", func() { + testee := newPluginConstructor(testPluginType(), newTestPluginImplErr) + factory, err := testee.NewFactory(testPluginNoErrFactoryType(), nil) + Expect(err).NotTo(HaveOccurred()) + f, ok := factory.(func() testPlugin) + Expect(ok).To(BeTrue()) + plugin := f() + expectConfigValue(plugin, testInitValue) + }) + + It("new factory config", func() { + testee := newPluginConstructor(testPluginType(), newTestPluginImplConf) + factory, err := testee.NewFactory(testPluginNoErrFactoryType(), confToGetMaybe(newTestDefaultConf())) Expect(err).NotTo(HaveOccurred()) + f, ok := factory.(func() testPlugin) + Expect(ok).To(BeTrue()) + plugin := f() expectConfigValue(plugin, testDefaultValue) }) + It("new factory, get config failed", func() { + testee := newPluginConstructor(testPluginType(), newTestPluginImplConf) + factory, err := testee.NewFactory(testPluginFactoryType(), errToGetMaybe(testConfigurationFailedErr)) + Expect(err).NotTo(HaveOccurred()) + f, ok := factory.(func() (testPlugin, error)) + Expect(ok).To(BeTrue()) + plugin, err := f() + Expect(err).To(Equal(testConfigurationFailedErr)) + Expect(plugin).To(BeNil()) + }) + + It("new factory no err, get config failed, throw panic", func() { + testee := newPluginConstructor(testPluginType(), newTestPluginImplConf) + factory, err := testee.NewFactory(testPluginNoErrFactoryType(), errToGetMaybe(testConfigurationFailedErr)) + Expect(err).NotTo(HaveOccurred()) + f, ok := factory.(func() testPlugin) + Expect(ok).To(BeTrue()) + func() { + defer func() { + r := recover() + Expect(r).To(Equal(testConfigurationFailedErr)) + }() + f() + }() + }) + + It("new factory panic on trim non nil err", func() { + testee := newPluginConstructor(testPluginType(), newTestPluginImplErrFailed) + factory, err := testee.NewFactory(testPluginNoErrFactoryType(), nil) + Expect(err).NotTo(HaveOccurred()) + f, ok := factory.(func() testPlugin) + Expect(ok).To(BeTrue()) + func() { + defer func() { + r := recover() + Expect(r).To(Equal(testPluginCreateFailedErr)) + }() + f() + }() + }) + }) + +func expectSameFunc(f1, f2 interface{}) { + s1 := fmt.Sprint(f1) + s2 := fmt.Sprint(f2) + Expect(s1).To(Equal(s2)) +} diff --git a/core/plugin/internal/ptest/plugin.go b/core/plugin/internal/ptest/plugin.go new file mode 100644 index 000000000..ddbc28ade --- /dev/null +++ b/core/plugin/internal/ptest/plugin.go @@ -0,0 +1,6 @@ +// Copyright (c) 2017 Yandex LLC. All rights reserved. +// Use of this source code is governed by a MPL 2.0 +// license that can be found in the LICENSE file. +// Author: Vladimir Skipor + +package ptest diff --git a/core/plugin/plugin_suite_test.go b/core/plugin/plugin_suite_test.go index 2487080c5..2f12a4529 100644 --- a/core/plugin/plugin_suite_test.go +++ b/core/plugin/plugin_suite_test.go @@ -1,3 +1,8 @@ +// Copyright (c) 2017 Yandex LLC. All rights reserved. +// Use of this source code is governed by a MPL 2.0 +// license that can be found in the LICENSE file. +// Author: Vladimir Skipor + package plugin import ( @@ -7,6 +12,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/pkg/errors" "github.com/yandex/pandora/core/config" "github.com/yandex/pandora/lib/testutil" ) @@ -47,15 +53,16 @@ type testPlugin interface { func testPluginType() reflect.Type { return reflect.TypeOf((*testPlugin)(nil)).Elem() } func testPluginImplType() reflect.Type { return reflect.TypeOf((*testPluginImpl)(nil)).Elem() } + func testPluginFactoryType() reflect.Type { return reflect.TypeOf(func() (testPlugin, error) { panic("") }) } -func testPluginImplFactoryType() reflect.Type { - return reflect.TypeOf(func() (*testPluginImpl, error) { panic("") }) -} func testPluginNoErrFactoryType() reflect.Type { return reflect.TypeOf(func() testPlugin { panic("") }) } +func testPluginFactoryConfigType() reflect.Type { + return reflect.TypeOf(func(testPluginConfig) (testPlugin, error) { panic("") }) +} type testPluginImpl struct{ Value string } @@ -72,8 +79,19 @@ func newTestPluginImplPtrConf(c *testPluginConfig) *testPluginImpl { return &testPluginImpl{c.Value} } -func newTestPluginDefaultConf() testPluginConfig { return testPluginConfig{testDefaultValue} } -func newTestPluginDefaultPtrConf() *testPluginConfig { return &testPluginConfig{testDefaultValue} } +func newTestPluginImplErr() (*testPluginImpl, error) { + return &testPluginImpl{Value: testInitValue}, nil +} + +var testPluginCreateFailedErr = errors.New("test plugin create failed") +var testConfigurationFailedErr = errors.New("test plugin configuration failed") + +func newTestPluginImplErrFailed() (*testPluginImpl, error) { + return nil, testPluginCreateFailedErr +} + +func newTestDefaultConf() testPluginConfig { return testPluginConfig{testDefaultValue} } +func newTestDefaultPtrConf() *testPluginConfig { return &testPluginConfig{testDefaultValue} } func fillTestPluginConf(conf interface{}) error { return config.Decode(map[string]interface{}{"Value": testFilledValue}, conf) diff --git a/core/plugin/plugin_test.go b/core/plugin/plugin_test.go index 7179b5199..e4fb80a8c 100644 --- a/core/plugin/plugin_test.go +++ b/core/plugin/plugin_test.go @@ -1,3 +1,8 @@ +// Copyright (c) 2017 Yandex LLC. All rights reserved. +// Use of this source code is governed by a MPL 2.0 +// license that can be found in the LICENSE file. +// Author: Vladimir Skipor + package plugin import ( diff --git a/core/plugin/registry_test.go b/core/plugin/registry_test.go index f8e7d050e..9759ababc 100644 --- a/core/plugin/registry_test.go +++ b/core/plugin/registry_test.go @@ -51,9 +51,9 @@ var _ = Describe("new default config container", func() { Entry("no default ptr config", newTestPluginImplPtrConf), Entry("default config", - newTestPluginImplConf, newTestPluginDefaultConf), + newTestPluginImplConf, newTestDefaultConf), Entry("default ptr config", - newTestPluginImplPtrConf, newTestPluginDefaultPtrConf), + newTestPluginImplPtrConf, newTestDefaultPtrConf), ) It("fill no config failed", func() { @@ -182,11 +182,11 @@ var _ = Describe("new", func() { Expect(testNewOk()).To(Equal("")) }) It("default", func() { - r.testRegister(newTestPluginImplConf, newTestPluginDefaultConf) + r.testRegister(newTestPluginImplConf, newTestDefaultConf) Expect(testNewOk()).To(Equal(testDefaultValue)) }) It("fill conf default", func() { - r.testRegister(newTestPluginImplConf, newTestPluginDefaultConf) + r.testRegister(newTestPluginImplConf, newTestDefaultConf) Expect("conf").To(Equal(testNewOk(fillTestPluginConf))) }) It("fill conf no default", func() { @@ -260,7 +260,7 @@ var _ = Describe("decode", func() { }) }) - r.Register(testPluginType(), "my-plugin", newTestPluginImplConf, newTestPluginDefaultConf) + r.Register(testPluginType(), "my-plugin", newTestPluginImplConf, newTestDefaultConf) input := map[string]interface{}{ "plugin": map[string]interface{}{ nameKey: "my-plugin", From 58818d6ba846670226fc6f9b186fc92d6d32bc1e Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Sat, 30 Sep 2017 19:35:42 +0300 Subject: [PATCH 08/15] Move plugin pkg test samples to ptest_test.go --- core/plugin/constructor_test.go | 84 ++++++++--------- core/plugin/internal/ptest/plugin.go | 6 -- core/plugin/plugin_suite_test.go | 91 ------------------ core/plugin/plugin_test.go | 18 ++-- core/plugin/ptest_test.go | 106 +++++++++++++++++++++ core/plugin/registry_test.go | 134 +++++++++++++-------------- 6 files changed, 224 insertions(+), 215 deletions(-) delete mode 100644 core/plugin/internal/ptest/plugin.go create mode 100644 core/plugin/ptest_test.go diff --git a/core/plugin/constructor_test.go b/core/plugin/constructor_test.go index ccdf07bb7..de9db876a 100644 --- a/core/plugin/constructor_test.go +++ b/core/plugin/constructor_test.go @@ -26,13 +26,13 @@ var _ = Describe("plugin constructor", func() { Entry("not func", errorType, errors.New("that is not constructor")), Entry("not implements", - errorType, newTestPluginImpl), + errorType, ptestNewImpl), Entry("too many args", - testPluginType(), func(_, _ testPluginConfig) testPlugin { panic("") }), + ptestType(), func(_, _ ptestConfig) ptestPlugin { panic("") }), Entry("too many return valued", - testPluginType(), func() (_ testPlugin, _, _ error) { panic("") }), + ptestType(), func() (_ ptestPlugin, _, _ error) { panic("") }), Entry("second return value is not error", - testPluginType(), func() (_, _ testPlugin) { panic("") }), + ptestType(), func() (_, _ ptestPlugin) { panic("") }), ) confToMaybe := func(conf interface{}) []reflect.Value { if conf != nil { @@ -54,110 +54,110 @@ var _ = Describe("plugin constructor", func() { } It("new plugin", func() { - testee := newPluginConstructor(testPluginType(), newTestPlugin) + testee := newPluginConstructor(ptestType(), ptestNew) plugin, err := testee.NewPlugin(nil) Expect(err).NotTo(HaveOccurred()) - expectConfigValue(plugin, testInitValue) + ptestExpectConfigValue(plugin, ptestInitValue) }) It("new config plugin ", func() { - testee := newPluginConstructor(testPluginType(), newTestPluginImplConf) - plugin, err := testee.NewPlugin(confToMaybe(newTestDefaultConf())) + testee := newPluginConstructor(ptestType(), ptestNewImplConf) + plugin, err := testee.NewPlugin(confToMaybe(ptestDefaultConf())) Expect(err).NotTo(HaveOccurred()) - expectConfigValue(plugin, testDefaultValue) + ptestExpectConfigValue(plugin, ptestDefaultValue) }) It("new plugin failed", func() { - testee := newPluginConstructor(testPluginType(), newTestPluginImplErrFailed) + testee := newPluginConstructor(ptestType(), ptestNewImplErrFailed) plugin, err := testee.NewPlugin(nil) - Expect(err).To(Equal(testPluginCreateFailedErr)) + Expect(err).To(Equal(ptestCreateFailedErr)) Expect(plugin).To(BeNil()) }) It("new factory from conversion", func() { - testee := newPluginConstructor(testPluginType(), newTestPlugin) - factory, err := testee.NewFactory(testPluginNoErrFactoryType(), nil) + testee := newPluginConstructor(ptestType(), ptestNew) + factory, err := testee.NewFactory(ptestFactoryNoErrType(), nil) Expect(err).NotTo(HaveOccurred()) - expectSameFunc(factory, newTestPlugin) + expectSameFunc(factory, ptestNew) }) It("new factory from new impl", func() { - testee := newPluginConstructor(testPluginType(), newTestPluginImpl) - factory, err := testee.NewFactory(testPluginNoErrFactoryType(), nil) + testee := newPluginConstructor(ptestType(), ptestNewImpl) + factory, err := testee.NewFactory(ptestFactoryNoErrType(), nil) Expect(err).NotTo(HaveOccurred()) - f, ok := factory.(func() testPlugin) + f, ok := factory.(func() ptestPlugin) Expect(ok).To(BeTrue()) plugin := f() - expectConfigValue(plugin, testInitValue) + ptestExpectConfigValue(plugin, ptestInitValue) }) It("new factory add err", func() { - testee := newPluginConstructor(testPluginType(), newTestPlugin) - factory, err := testee.NewFactory(testPluginFactoryType(), nil) + testee := newPluginConstructor(ptestType(), ptestNew) + factory, err := testee.NewFactory(ptestFactoryType(), nil) Expect(err).NotTo(HaveOccurred()) - f, ok := factory.(func() (testPlugin, error)) + f, ok := factory.(func() (ptestPlugin, error)) Expect(ok).To(BeTrue()) plugin, err := f() Expect(err).NotTo(HaveOccurred()) - expectConfigValue(plugin, testInitValue) + ptestExpectConfigValue(plugin, ptestInitValue) }) It("new factory trim nil err", func() { - testee := newPluginConstructor(testPluginType(), newTestPluginImplErr) - factory, err := testee.NewFactory(testPluginNoErrFactoryType(), nil) + testee := newPluginConstructor(ptestType(), ptestNewImplErr) + factory, err := testee.NewFactory(ptestFactoryNoErrType(), nil) Expect(err).NotTo(HaveOccurred()) - f, ok := factory.(func() testPlugin) + f, ok := factory.(func() ptestPlugin) Expect(ok).To(BeTrue()) plugin := f() - expectConfigValue(plugin, testInitValue) + ptestExpectConfigValue(plugin, ptestInitValue) }) It("new factory config", func() { - testee := newPluginConstructor(testPluginType(), newTestPluginImplConf) - factory, err := testee.NewFactory(testPluginNoErrFactoryType(), confToGetMaybe(newTestDefaultConf())) + testee := newPluginConstructor(ptestType(), ptestNewImplConf) + factory, err := testee.NewFactory(ptestFactoryNoErrType(), confToGetMaybe(ptestDefaultConf())) Expect(err).NotTo(HaveOccurred()) - f, ok := factory.(func() testPlugin) + f, ok := factory.(func() ptestPlugin) Expect(ok).To(BeTrue()) plugin := f() - expectConfigValue(plugin, testDefaultValue) + ptestExpectConfigValue(plugin, ptestDefaultValue) }) It("new factory, get config failed", func() { - testee := newPluginConstructor(testPluginType(), newTestPluginImplConf) - factory, err := testee.NewFactory(testPluginFactoryType(), errToGetMaybe(testConfigurationFailedErr)) + testee := newPluginConstructor(ptestType(), ptestNewImplConf) + factory, err := testee.NewFactory(ptestFactoryType(), errToGetMaybe(ptestConfigurationFailedErr)) Expect(err).NotTo(HaveOccurred()) - f, ok := factory.(func() (testPlugin, error)) + f, ok := factory.(func() (ptestPlugin, error)) Expect(ok).To(BeTrue()) plugin, err := f() - Expect(err).To(Equal(testConfigurationFailedErr)) + Expect(err).To(Equal(ptestConfigurationFailedErr)) Expect(plugin).To(BeNil()) }) It("new factory no err, get config failed, throw panic", func() { - testee := newPluginConstructor(testPluginType(), newTestPluginImplConf) - factory, err := testee.NewFactory(testPluginNoErrFactoryType(), errToGetMaybe(testConfigurationFailedErr)) + testee := newPluginConstructor(ptestType(), ptestNewImplConf) + factory, err := testee.NewFactory(ptestFactoryNoErrType(), errToGetMaybe(ptestConfigurationFailedErr)) Expect(err).NotTo(HaveOccurred()) - f, ok := factory.(func() testPlugin) + f, ok := factory.(func() ptestPlugin) Expect(ok).To(BeTrue()) func() { defer func() { r := recover() - Expect(r).To(Equal(testConfigurationFailedErr)) + Expect(r).To(Equal(ptestConfigurationFailedErr)) }() f() }() }) It("new factory panic on trim non nil err", func() { - testee := newPluginConstructor(testPluginType(), newTestPluginImplErrFailed) - factory, err := testee.NewFactory(testPluginNoErrFactoryType(), nil) + testee := newPluginConstructor(ptestType(), ptestNewImplErrFailed) + factory, err := testee.NewFactory(ptestFactoryNoErrType(), nil) Expect(err).NotTo(HaveOccurred()) - f, ok := factory.(func() testPlugin) + f, ok := factory.(func() ptestPlugin) Expect(ok).To(BeTrue()) func() { defer func() { r := recover() - Expect(r).To(Equal(testPluginCreateFailedErr)) + Expect(r).To(Equal(ptestCreateFailedErr)) }() f() }() diff --git a/core/plugin/internal/ptest/plugin.go b/core/plugin/internal/ptest/plugin.go deleted file mode 100644 index ddbc28ade..000000000 --- a/core/plugin/internal/ptest/plugin.go +++ /dev/null @@ -1,6 +0,0 @@ -// Copyright (c) 2017 Yandex LLC. All rights reserved. -// Use of this source code is governed by a MPL 2.0 -// license that can be found in the LICENSE file. -// Author: Vladimir Skipor - -package ptest diff --git a/core/plugin/plugin_suite_test.go b/core/plugin/plugin_suite_test.go index 2f12a4529..6eda14d0a 100644 --- a/core/plugin/plugin_suite_test.go +++ b/core/plugin/plugin_suite_test.go @@ -6,14 +6,11 @@ package plugin import ( - "reflect" "testing" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/pkg/errors" - "github.com/yandex/pandora/core/config" "github.com/yandex/pandora/lib/testutil" ) @@ -22,91 +19,3 @@ func TestPlugin(t *testing.T) { testutil.ReplaceGlobalLogger() RunSpecs(t, "Plugin Suite") } - -const ( - testPluginName = "test_name" - testDefaultValue = "default" - testInitValue = "init" - testFilledValue = "conf" -) - -func (r *Registry) testRegister(newPluginImpl interface{}, newDefaultConfigOptional ...interface{}) { - r.Register(testPluginType(), testPluginName, newPluginImpl, newDefaultConfigOptional...) -} - -func (r *Registry) testNew(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { - return r.New(testPluginType(), testPluginName, fillConfOptional...) -} - -func (r *Registry) testNewFactory(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { - factory, err := r.NewFactory(testPluginFactoryType(), testPluginName, fillConfOptional...) - if err != nil { - return - } - typedFactory := factory.(func() (testPlugin, error)) - return typedFactory() -} - -type testPlugin interface { - DoSomething() -} - -func testPluginType() reflect.Type { return reflect.TypeOf((*testPlugin)(nil)).Elem() } -func testPluginImplType() reflect.Type { return reflect.TypeOf((*testPluginImpl)(nil)).Elem() } - -func testPluginFactoryType() reflect.Type { - return reflect.TypeOf(func() (testPlugin, error) { panic("") }) -} -func testPluginNoErrFactoryType() reflect.Type { - return reflect.TypeOf(func() testPlugin { panic("") }) -} -func testPluginFactoryConfigType() reflect.Type { - return reflect.TypeOf(func(testPluginConfig) (testPlugin, error) { panic("") }) -} - -type testPluginImpl struct{ Value string } - -func (p *testPluginImpl) DoSomething() {} - -var _ testPlugin = (*testPluginImpl)(nil) - -type testPluginConfig struct{ Value string } - -func newTestPlugin() testPlugin { return newTestPluginImpl() } -func newTestPluginImpl() *testPluginImpl { return &testPluginImpl{Value: testInitValue} } -func newTestPluginImplConf(c testPluginConfig) *testPluginImpl { return &testPluginImpl{c.Value} } -func newTestPluginImplPtrConf(c *testPluginConfig) *testPluginImpl { - return &testPluginImpl{c.Value} -} - -func newTestPluginImplErr() (*testPluginImpl, error) { - return &testPluginImpl{Value: testInitValue}, nil -} - -var testPluginCreateFailedErr = errors.New("test plugin create failed") -var testConfigurationFailedErr = errors.New("test plugin configuration failed") - -func newTestPluginImplErrFailed() (*testPluginImpl, error) { - return nil, testPluginCreateFailedErr -} - -func newTestDefaultConf() testPluginConfig { return testPluginConfig{testDefaultValue} } -func newTestDefaultPtrConf() *testPluginConfig { return &testPluginConfig{testDefaultValue} } - -func fillTestPluginConf(conf interface{}) error { - return config.Decode(map[string]interface{}{"Value": testFilledValue}, conf) -} - -func expectConfigValue(conf interface{}, val string) { - conf.(confChecker).expectValue(val) -} - -type confChecker interface { - expectValue(string) -} - -var _ confChecker = testPluginConfig{} -var _ confChecker = &testPluginImpl{} - -func (c testPluginConfig) expectValue(val string) { Expect(c.Value).To(Equal(val)) } -func (p *testPluginImpl) expectValue(val string) { Expect(p.Value).To(Equal(val)) } diff --git a/core/plugin/plugin_test.go b/core/plugin/plugin_test.go index e4fb80a8c..f59fa82e2 100644 --- a/core/plugin/plugin_test.go +++ b/core/plugin/plugin_test.go @@ -12,24 +12,24 @@ import ( var _ = Describe("Default registry", func() { BeforeEach(func() { - Register(testPluginType(), testPluginName, newTestPluginImpl) + Register(ptestType(), ptestPluginName, ptestNewImpl) }) AfterEach(func() { defaultRegistry = NewRegistry() }) It("lookup", func() { - Expect(Lookup(testPluginType())).To(BeTrue()) + Expect(Lookup(ptestType())).To(BeTrue()) }) It("lookup factory", func() { - Expect(LookupFactory(testPluginFactoryType())).To(BeTrue()) + Expect(LookupFactory(ptestFactoryType())).To(BeTrue()) }) It("new", func() { - plugin, err := New(testPluginType(), testPluginName) + plugin, err := New(ptestType(), ptestPluginName) Expect(err).NotTo(HaveOccurred()) Expect(plugin).NotTo(BeNil()) }) It("new factory", func() { - pluginFactory, err := NewFactory(testPluginFactoryType(), testPluginName) + pluginFactory, err := NewFactory(ptestFactoryType(), ptestPluginName) Expect(err).NotTo(HaveOccurred()) Expect(pluginFactory).NotTo(BeNil()) }) @@ -37,12 +37,12 @@ var _ = Describe("Default registry", func() { var _ = Describe("type helpers", func() { It("ptr type", func() { - var plugin testPlugin - Expect(PtrType(&plugin)).To(Equal(testPluginType())) + var plugin ptestPlugin + Expect(PtrType(&plugin)).To(Equal(ptestType())) }) It("factory plugin type ok", func() { - factoryPlugin, ok := FactoryPluginType(testPluginFactoryType()) + factoryPlugin, ok := FactoryPluginType(ptestFactoryType()) Expect(ok).To(BeTrue()) - Expect(factoryPlugin).To(Equal(testPluginType())) + Expect(factoryPlugin).To(Equal(ptestType())) }) }) diff --git a/core/plugin/ptest_test.go b/core/plugin/ptest_test.go new file mode 100644 index 000000000..1dfd08a28 --- /dev/null +++ b/core/plugin/ptest_test.go @@ -0,0 +1,106 @@ +// Copyright (c) 2017 Yandex LLC. All rights reserved. +// Use of this source code is governed by a MPL 2.0 +// license that can be found in the LICENSE file. +// Author: Vladimir Skipor + +package plugin + +import ( + "reflect" + + . "github.com/onsi/gomega" + "github.com/pkg/errors" + + "github.com/yandex/pandora/core/config" +) + +// ptest contains samples and utils for testing plugin pkg + +const ( + ptestPluginName = "ptest_name" + + ptestInitValue = "ptest_INITIAL" + ptestDefaultValue = "ptest_DEFAULT_CONFIG" + ptestFilledValue = "ptest_FILLED" +) + +func (r *Registry) ptestRegister(newPluginImpl interface{}, newDefaultConfigOptional ...interface{}) { + r.Register(ptestType(), ptestPluginName, newPluginImpl, newDefaultConfigOptional...) +} + +func (r *Registry) ptestNew(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { + return r.New(ptestType(), ptestPluginName, fillConfOptional...) +} + +func (r *Registry) ptestNewFactory(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { + factory, err := r.NewFactory(ptestFactoryType(), ptestPluginName, fillConfOptional...) + if err != nil { + return + } + typedFactory := factory.(func() (ptestPlugin, error)) + return typedFactory() +} + +type ptestPlugin interface { + DoSomething() +} + +func ptestType() reflect.Type { return reflect.TypeOf((*ptestPlugin)(nil)).Elem() } +func ptestImplType() reflect.Type { return reflect.TypeOf((*ptestImpl)(nil)).Elem() } + +func ptestFactoryType() reflect.Type { + return reflect.TypeOf(func() (ptestPlugin, error) { panic("") }) +} +func ptestFactoryNoErrType() reflect.Type { + return reflect.TypeOf(func() ptestPlugin { panic("") }) +} +func ptestFactoryConfigType() reflect.Type { + return reflect.TypeOf(func(ptestConfig) (ptestPlugin, error) { panic("") }) +} + +type ptestImpl struct{ Value string } + +func (p *ptestImpl) DoSomething() {} + +var _ ptestPlugin = (*ptestImpl)(nil) + +type ptestConfig struct{ Value string } + +func ptestNew() ptestPlugin { return ptestNewImpl() } +func ptestNewImpl() *ptestImpl { return &ptestImpl{Value: ptestInitValue} } +func ptestNewImplConf(c ptestConfig) *ptestImpl { return &ptestImpl{c.Value} } +func ptestNewImplPtrConf(c *ptestConfig) *ptestImpl { + return &ptestImpl{c.Value} +} + +func ptestNewImplErr() (*ptestImpl, error) { + return &ptestImpl{Value: ptestInitValue}, nil +} + +var ptestCreateFailedErr = errors.New("test plugin create failed") +var ptestConfigurationFailedErr = errors.New("test plugin configuration failed") + +func ptestNewImplErrFailed() (*ptestImpl, error) { + return nil, ptestCreateFailedErr +} + +func ptestDefaultConf() ptestConfig { return ptestConfig{ptestDefaultValue} } +func ptestNewDefaultPtrConf() *ptestConfig { return &ptestConfig{ptestDefaultValue} } + +func ptestFillConf(conf interface{}) error { + return config.Decode(map[string]interface{}{"Value": ptestFilledValue}, conf) +} + +func ptestExpectConfigValue(conf interface{}, val string) { + conf.(ptestConfChecker).expectConfValue(val) +} + +type ptestConfChecker interface { + expectConfValue(string) +} + +var _ ptestConfChecker = ptestConfig{} +var _ ptestConfChecker = &ptestImpl{} + +func (c ptestConfig) expectConfValue(val string) { Expect(c.Value).To(Equal(val)) } +func (p *ptestImpl) expectConfValue(val string) { Expect(p.Value).To(Equal(val)) } diff --git a/core/plugin/registry_test.go b/core/plugin/registry_test.go index 9759ababc..8ba3e48d3 100644 --- a/core/plugin/registry_test.go +++ b/core/plugin/registry_test.go @@ -24,41 +24,41 @@ var _ = Describe("new default config container", func() { newDefaultConfigContainer(reflect.TypeOf(constructor), newDefaultConfig) }, Entry("invalid type", - func(int) testPlugin { return nil }), + func(int) ptestPlugin { return nil }), Entry("invalid ptr type", - func(*int) testPlugin { return nil }), + func(*int) ptestPlugin { return nil }), Entry("to many args", - func(_, _ testPluginConfig) testPlugin { return nil }), + func(_, _ ptestConfig) ptestPlugin { return nil }), Entry("default without config", - func() testPlugin { return nil }, func() *testPluginConfig { return nil }), + func() ptestPlugin { return nil }, func() *ptestConfig { return nil }), Entry("invalid default config", - func(testPluginConfig) testPlugin { return nil }, func() *testPluginConfig { return nil }), + func(ptestConfig) ptestPlugin { return nil }, func() *ptestConfig { return nil }), Entry("default config accepts args", - func(*testPluginConfig) testPlugin { return nil }, func(int) *testPluginConfig { return nil }), + func(*ptestConfig) ptestPlugin { return nil }, func(int) *ptestConfig { return nil }), ) DescribeTable("expectation ok", func(constructor interface{}, newDefaultConfigOptional ...interface{}) { newDefaultConfig := getNewDefaultConfig(newDefaultConfigOptional) container := newDefaultConfigContainer(reflect.TypeOf(constructor), newDefaultConfig) - conf, err := container.Get(fillTestPluginConf) + conf, err := container.Get(ptestFillConf) Expect(err).NotTo(HaveOccurred()) Expect(conf).To(HaveLen(1)) - expectConfigValue(conf[0].Interface(), testFilledValue) + ptestExpectConfigValue(conf[0].Interface(), ptestFilledValue) }, Entry("no default config", - newTestPluginImplConf), + ptestNewImplConf), Entry("no default ptr config", - newTestPluginImplPtrConf), + ptestNewImplPtrConf), Entry("default config", - newTestPluginImplConf, newTestDefaultConf), + ptestNewImplConf, ptestDefaultConf), Entry("default ptr config", - newTestPluginImplPtrConf, newTestDefaultPtrConf), + ptestNewImplPtrConf, ptestNewDefaultPtrConf), ) It("fill no config failed", func() { - container := newDefaultConfigContainer(testPluginFactoryType(), nil) - _, err := container.Get(fillTestPluginConf) + container := newDefaultConfigContainer(ptestFactoryType(), nil) + _, err := container.Get(ptestFillConf) Expect(err).To(HaveOccurred()) }) }) @@ -69,27 +69,27 @@ var _ = DescribeTable("register valid", newDefaultConfigOptional ...interface{}, ) { Expect(func() { - NewRegistry().testRegister(newPluginImpl, newDefaultConfigOptional...) + NewRegistry().ptestRegister(newPluginImpl, newDefaultConfigOptional...) }).NotTo(Panic()) }, Entry("return impl", - func() *testPluginImpl { return nil }), + func() *ptestImpl { return nil }), Entry("return interface", - func() testPlugin { return nil }), + func() ptestPlugin { return nil }), Entry("super interface", func() interface { io.Writer - testPlugin + ptestPlugin } { return nil }), Entry("struct config", - func(testPluginConfig) testPlugin { return nil }), + func(ptestConfig) ptestPlugin { return nil }), Entry("struct ptr config", - func(*testPluginConfig) testPlugin { return nil }), + func(*ptestConfig) ptestPlugin { return nil }), Entry("default config", - func(*testPluginConfig) testPlugin { return nil }, - func() *testPluginConfig { return nil }), + func(*ptestConfig) ptestPlugin { return nil }, + func() *ptestConfig { return nil }), ) var _ = DescribeTable("register invalid", @@ -99,40 +99,40 @@ var _ = DescribeTable("register invalid", ) { Expect(func() { defer recoverExpectationFail() - NewRegistry().testRegister(newPluginImpl, newDefaultConfigOptional...) + NewRegistry().ptestRegister(newPluginImpl, newDefaultConfigOptional...) }).NotTo(Panic()) }, Entry("return not impl", - func() testPluginImpl { panic("") }), + func() ptestImpl { panic("") }), Entry("invalid config type", - func(int) testPlugin { return nil }), + func(int) ptestPlugin { return nil }), Entry("invalid config ptr type", - func(*int) testPlugin { return nil }), + func(*int) ptestPlugin { return nil }), Entry("to many args", - func(_, _ testPluginConfig) testPlugin { return nil }), + func(_, _ ptestConfig) ptestPlugin { return nil }), Entry("default without config", - func() testPlugin { return nil }, func() *testPluginConfig { return nil }), + func() ptestPlugin { return nil }, func() *ptestConfig { return nil }), Entry("extra default config", - func(*testPluginConfig) testPlugin { return nil }, func() *testPluginConfig { return nil }, 0), + func(*ptestConfig) ptestPlugin { return nil }, func() *ptestConfig { return nil }, 0), Entry("invalid default config", - func(testPluginConfig) testPlugin { return nil }, func() *testPluginConfig { return nil }), + func(ptestConfig) ptestPlugin { return nil }, func() *ptestConfig { return nil }), Entry("default config accepts args", - func(*testPluginConfig) testPlugin { return nil }, func(int) *testPluginConfig { return nil }), + func(*ptestConfig) ptestPlugin { return nil }, func(int) *ptestConfig { return nil }), ) var _ = Describe("registry", func() { It("register name collision panics", func() { r := NewRegistry() - r.testRegister(newTestPluginImpl) + r.ptestRegister(ptestNewImpl) defer recoverExpectationFail() - r.testRegister(newTestPluginImpl) + r.ptestRegister(ptestNewImpl) }) It("lookup", func() { r := NewRegistry() - r.testRegister(newTestPluginImpl) - Expect(r.Lookup(testPluginType())).To(BeTrue()) + r.ptestRegister(ptestNewImpl) + Expect(r.Lookup(ptestType())).To(BeTrue()) Expect(r.Lookup(reflect.TypeOf(0))).To(BeFalse()) - Expect(r.Lookup(reflect.TypeOf(&testPluginImpl{}))).To(BeFalse()) + Expect(r.Lookup(reflect.TypeOf(&ptestImpl{}))).To(BeFalse()) Expect(r.Lookup(reflect.TypeOf((*io.Writer)(nil)).Elem())).To(BeFalse()) }) @@ -146,24 +146,24 @@ var _ = Describe("new", func() { testNewOk = func(fillConfOptional ...func(conf interface{}) error) (pluginVal string) { plugin, err := testNew(r, fillConfOptional...) Expect(err).NotTo(HaveOccurred()) - return plugin.(*testPluginImpl).Value + return plugin.(*ptestImpl).Value } ) BeforeEach(func() { r = NewRegistry() }) runTestCases := func() { It("no conf", func() { - r.testRegister(newTestPluginImpl) - Expect(testNewOk()).To(Equal(testInitValue)) + r.ptestRegister(ptestNewImpl) + Expect(testNewOk()).To(Equal(ptestInitValue)) }) It("nil error", func() { - r.testRegister(func() (testPlugin, error) { - return newTestPluginImpl(), nil + r.ptestRegister(func() (ptestPlugin, error) { + return ptestNewImpl(), nil }) - Expect(testNewOk()).To(Equal(testInitValue)) + Expect(testNewOk()).To(Equal(ptestInitValue)) }) It("non-nil error", func() { expectedErr := errors.New("fill conf err") - r.testRegister(func() (testPlugin, error) { + r.ptestRegister(func() (ptestPlugin, error) { return nil, expectedErr }) _, err := testNew(r) @@ -172,56 +172,56 @@ var _ = Describe("new", func() { Expect(expectedErr).To(Equal(err)) }) It("no conf, fill conf error", func() { - r.testRegister(newTestPluginImpl) + r.ptestRegister(ptestNewImpl) expectedErr := errors.New("fill conf err") _, err := testNew(r, func(_ interface{}) error { return expectedErr }) Expect(expectedErr).To(Equal(err)) }) It("no default", func() { - r.testRegister(func(c testPluginConfig) *testPluginImpl { return &testPluginImpl{c.Value} }) + r.ptestRegister(func(c ptestConfig) *ptestImpl { return &ptestImpl{c.Value} }) Expect(testNewOk()).To(Equal("")) }) It("default", func() { - r.testRegister(newTestPluginImplConf, newTestDefaultConf) - Expect(testNewOk()).To(Equal(testDefaultValue)) + r.ptestRegister(ptestNewImplConf, ptestDefaultConf) + Expect(testNewOk()).To(Equal(ptestDefaultValue)) }) It("fill conf default", func() { - r.testRegister(newTestPluginImplConf, newTestDefaultConf) - Expect("conf").To(Equal(testNewOk(fillTestPluginConf))) + r.ptestRegister(ptestNewImplConf, ptestDefaultConf) + Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) }) It("fill conf no default", func() { - r.testRegister(newTestPluginImplConf) - Expect("conf").To(Equal(testNewOk(fillTestPluginConf))) + r.ptestRegister(ptestNewImplConf) + Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) }) It("fill ptr conf no default", func() { - r.testRegister(newTestPluginImplPtrConf) - Expect("conf").To(Equal(testNewOk(fillTestPluginConf))) + r.ptestRegister(ptestNewImplPtrConf) + Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) }) It("no default ptr conf not nil", func() { - r.testRegister(newTestPluginImplPtrConf) + r.ptestRegister(ptestNewImplPtrConf) Expect("").To(Equal(testNewOk())) }) It("nil default, conf not nil", func() { - r.testRegister(newTestPluginImplPtrConf, func() *testPluginConfig { return nil }) - Expect("").To(Equal(testNewOk())) + r.ptestRegister(ptestNewImplPtrConf, func() *ptestConfig { return nil }) + Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) }) It("fill nil default", func() { - r.testRegister(newTestPluginImplPtrConf, func() *testPluginConfig { return nil }) - Expect("conf").To(Equal(testNewOk(fillTestPluginConf))) + r.ptestRegister(ptestNewImplPtrConf, func() *ptestConfig { return nil }) + Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) }) It("more than one fill conf panics", func() { - r.testRegister(newTestPluginImplPtrConf) + r.ptestRegister(ptestNewImplPtrConf) defer recoverExpectationFail() - testNew(r, fillTestPluginConf, fillTestPluginConf) + testNew(r, ptestFillConf, ptestFillConf) }) } Context("use New", func() { - BeforeEach(func() { testNew = (*Registry).testNew }) + BeforeEach(func() { testNew = (*Registry).ptestNew }) runTestCases() }) Context("use NewFactory", func() { - BeforeEach(func() { testNew = (*Registry).testNewFactory }) + BeforeEach(func() { testNew = (*Registry).ptestNewFactory }) runTestCases() }) @@ -260,21 +260,21 @@ var _ = Describe("decode", func() { }) }) - r.Register(testPluginType(), "my-plugin", newTestPluginImplConf, newTestDefaultConf) + r.Register(ptestType(), "my-plugin", ptestNewImplConf, ptestDefaultConf) input := map[string]interface{}{ "plugin": map[string]interface{}{ nameKey: "my-plugin", - "value": testFilledValue, + "value": ptestFilledValue, }, } type Config struct { - Plugin testPlugin + Plugin ptestPlugin } var conf Config err := decode(input, &conf) Expect(err).NotTo(HaveOccurred()) - actualValue := conf.Plugin.(*testPluginImpl).Value - Expect(actualValue).To(Equal(testFilledValue)) + actualValue := conf.Plugin.(*ptestImpl).Value + Expect(actualValue).To(Equal(ptestFilledValue)) }) }) From 169136ecaebdb6788856d7be2e9ddf023db52a15 Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Sat, 30 Sep 2017 21:48:25 +0300 Subject: [PATCH 09/15] Refactor plugin pkg tests --- core/plugin/constructor.go | 49 ++++-- core/plugin/constructor_test.go | 280 ++++++++++++++++++-------------- core/plugin/plugin_test.go | 6 +- core/plugin/ptest_test.go | 68 ++++---- core/plugin/registry_test.go | 28 ++-- 5 files changed, 238 insertions(+), 193 deletions(-) diff --git a/core/plugin/constructor.go b/core/plugin/constructor.go index 0f95b6c1f..734503d62 100644 --- a/core/plugin/constructor.go +++ b/core/plugin/constructor.go @@ -25,18 +25,8 @@ type constructor interface { } func newPluginConstructor(pluginType reflect.Type, newPlugin interface{}) *pluginConstructor { - newPluginType := reflect.TypeOf(newPlugin) - expect(newPluginType.Kind() == reflect.Func, "plugin constructor should be func") - expect(newPluginType.NumIn() <= 1, "plugin constructor should accept config or nothing") - expect(1 <= newPluginType.NumOut() && newPluginType.NumOut() <= 2, - "plugin constructor should return plugin implementation, and optionally error") - pluginImplType := newPluginType.Out(0) - expect(pluginImplType.Implements(pluginType), "plugin constructor should implement plugin interface") - if newPluginType.NumOut() == 2 { - expect(newPluginType.Out(1) == errorType, "plugin constructor should have no second return value, or it should be error") - } - newPluginVal := reflect.ValueOf(newPlugin) - return &pluginConstructor{pluginType, newPluginVal} + expectPluginConstructor(pluginType, reflect.TypeOf(newPlugin), true) + return &pluginConstructor{pluginType, reflect.ValueOf(newPlugin)} } // pluginConstructor is abstract constructor of some pluginType interface implementations @@ -88,11 +78,25 @@ func (c *pluginConstructor) NewFactory(factoryType reflect.Type, getMaybeConf fu // factory functions, using some func newFactory func. type factoryConstructor struct { pluginType reflect.Type - // newFactory type is func([config ]) (func() ( [, error])), - // where configType kind is struct or struct pointer. + // newFactory type is func([config ]) (func() ([, error])[, error), newFactory reflect.Value } +func newFactoryConstructor(pluginType reflect.Type, newFactory interface{}) *factoryConstructor { + newFactoryType := reflect.TypeOf(newFactory) + expect(newFactoryType.Kind() == reflect.Func, "factory constructor should be func") + expect(newFactoryType.NumIn() <= 1, "factory constructor should accept config or nothing") + + expect(1 <= newFactoryType.NumOut() && newFactoryType.NumOut() <= 2, + "factory constructor should return factory, and optionally error") + if newFactoryType.NumOut() == 2 { + expect(newFactoryType.Out(1) == errorType, "factory constructor should have no second return value, or it should be error") + } + factoryType := newFactoryType.Out(0) + expectPluginConstructor(pluginType, factoryType, false) + return &factoryConstructor{pluginType, reflect.ValueOf(newFactory)} +} + func (c *factoryConstructor) NewPlugin(maybeConf []reflect.Value) (plugin interface{}, err error) { factory, err := c.callNewFactory(maybeConf) if err != nil { @@ -136,6 +140,23 @@ func (c *factoryConstructor) callNewFactory(maybeConf []reflect.Value) (factory return factoryAndMaybeErr[0], err } +// expectPluginConstructor checks type expectations common for newPlugin, and factory, returned from newFactory. +func expectPluginConstructor(pluginType, factoryType reflect.Type, configAllowed bool) { + expect(factoryType.Kind() == reflect.Func, "plugin constructor should be func") + if configAllowed { + expect(factoryType.NumIn() <= 1, "plugin constructor should accept config or nothing") + } else { + expect(factoryType.NumIn() == 0, "plugin constructor returned from newFactory, shouldn't accept any arguments") + } + expect(1 <= factoryType.NumOut() && factoryType.NumOut() <= 2, + "plugin constructor should return plugin implementation, and optionally error") + pluginImplType := factoryType.Out(0) + expect(pluginImplType.Implements(pluginType), "plugin constructor should implement plugin interface") + if factoryType.NumOut() == 2 { + expect(factoryType.Out(1) == errorType, "plugin constructor should have no second return value, or it should be error") + } +} + // convertFactoryOutParam converts output params of some factory (newFactory) call, to required. func convertFactoryOutParam(pluginType reflect.Type, numOut int, out []reflect.Value) []reflect.Value { switch numOut { diff --git a/core/plugin/constructor_test.go b/core/plugin/constructor_test.go index de9db876a..83a8f2e10 100644 --- a/core/plugin/constructor_test.go +++ b/core/plugin/constructor_test.go @@ -6,9 +6,8 @@ package plugin import ( - "reflect" - "fmt" + "reflect" . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" @@ -18,152 +17,181 @@ import ( var _ = Describe("plugin constructor", func() { DescribeTable("expectations failed", - func(pluginType reflect.Type, newPlugin interface{}) { + func(newPlugin interface{}) { defer recoverExpectationFail() - newPluginConstructor(pluginType, newPlugin) - Expect(true) + newPluginConstructor(ptestType(), newPlugin) }, Entry("not func", - errorType, errors.New("that is not constructor")), + errors.New("that is not constructor")), Entry("not implements", - errorType, ptestNewImpl), + func() struct{} { panic("") }), Entry("too many args", - ptestType(), func(_, _ ptestConfig) ptestPlugin { panic("") }), + func(_, _ ptestConfig) ptestPlugin { panic("") }), Entry("too many return valued", - ptestType(), func() (_ ptestPlugin, _, _ error) { panic("") }), + func() (_ ptestPlugin, _, _ error) { panic("") }), Entry("second return value is not error", - ptestType(), func() (_, _ ptestPlugin) { panic("") }), + func() (_, _ ptestPlugin) { panic("") }), ) - confToMaybe := func(conf interface{}) []reflect.Value { - if conf != nil { - return []reflect.Value{reflect.ValueOf(conf)} - } - return nil - } - confToGetMaybe := func(conf interface{}) func() ([]reflect.Value, error) { - return func() ([]reflect.Value, error) { - return confToMaybe(conf), nil + Context("new plugin", func() { + newPlugin := func(newPlugin interface{}, maybeConf []reflect.Value) (interface{}, error) { + testee := newPluginConstructor(ptestType(), newPlugin) + return testee.NewPlugin(maybeConf) } - } - - errToGetMaybe := func(err error) func() ([]reflect.Value, error) { - return func() ([]reflect.Value, error) { - return nil, err - } - } - - It("new plugin", func() { - testee := newPluginConstructor(ptestType(), ptestNew) - plugin, err := testee.NewPlugin(nil) - Expect(err).NotTo(HaveOccurred()) - ptestExpectConfigValue(plugin, ptestInitValue) - }) - It("new config plugin ", func() { - testee := newPluginConstructor(ptestType(), ptestNewImplConf) - plugin, err := testee.NewPlugin(confToMaybe(ptestDefaultConf())) - Expect(err).NotTo(HaveOccurred()) - ptestExpectConfigValue(plugin, ptestDefaultValue) + It("", func() { + plugin, err := newPlugin(ptestNew, nil) + Expect(err).NotTo(HaveOccurred()) + ptestExpectConfigValue(plugin, ptestInitValue) + }) + + It("config", func() { + plugin, err := newPlugin(ptestNewConf, confToMaybe(ptestDefaultConf())) + Expect(err).NotTo(HaveOccurred()) + ptestExpectConfigValue(plugin, ptestDefaultValue) + }) + + It("failed", func() { + plugin, err := newPlugin(ptestNewErrFailing, nil) + Expect(err).To(Equal(ptestCreateFailedErr)) + Expect(plugin).To(BeNil()) + }) }) - It("new plugin failed", func() { - testee := newPluginConstructor(ptestType(), ptestNewImplErrFailed) - plugin, err := testee.NewPlugin(nil) - Expect(err).To(Equal(ptestCreateFailedErr)) - Expect(plugin).To(BeNil()) - }) - - It("new factory from conversion", func() { - testee := newPluginConstructor(ptestType(), ptestNew) - factory, err := testee.NewFactory(ptestFactoryNoErrType(), nil) - Expect(err).NotTo(HaveOccurred()) - expectSameFunc(factory, ptestNew) - }) + Context("new factory", func() { + newFactoryOK := func(newPlugin interface{}, factoryType reflect.Type, getMaybeConf func() ([]reflect.Value, error)) interface{} { + testee := newPluginConstructor(ptestType(), newPlugin) + factory, err := testee.NewFactory(factoryType, getMaybeConf) + Expect(err).NotTo(HaveOccurred()) + return factory + } - It("new factory from new impl", func() { - testee := newPluginConstructor(ptestType(), ptestNewImpl) - factory, err := testee.NewFactory(ptestFactoryNoErrType(), nil) - Expect(err).NotTo(HaveOccurred()) - f, ok := factory.(func() ptestPlugin) - Expect(ok).To(BeTrue()) - plugin := f() - ptestExpectConfigValue(plugin, ptestInitValue) - }) + It("from conversion", func() { + factory := newFactoryOK(ptestNew, ptestNewType(), nil) + expectSameFunc(factory, ptestNew) + }) + + It("from new impl", func() { + factory := newFactoryOK(ptestNewImpl, ptestNewType(), nil) + f, ok := factory.(func() ptestPlugin) + Expect(ok).To(BeTrue()) + plugin := f() + ptestExpectConfigValue(plugin, ptestInitValue) + }) + + It("add err", func() { + factory := newFactoryOK(ptestNew, ptestNewErrType(), nil) + f, ok := factory.(func() (ptestPlugin, error)) + Expect(ok).To(BeTrue()) + plugin, err := f() + Expect(err).NotTo(HaveOccurred()) + ptestExpectConfigValue(plugin, ptestInitValue) + }) + + It("trim nil err", func() { + factory := newFactoryOK(ptestNewErr, ptestNewType(), nil) + f, ok := factory.(func() ptestPlugin) + Expect(ok).To(BeTrue()) + plugin := f() + ptestExpectConfigValue(plugin, ptestInitValue) + }) + + It("config", func() { + factory := newFactoryOK(ptestNewConf, ptestNewType(), confToGetMaybe(ptestDefaultConf())) + f, ok := factory.(func() ptestPlugin) + Expect(ok).To(BeTrue()) + plugin := f() + ptestExpectConfigValue(plugin, ptestDefaultValue) + }) + + It("new factory, get config failed", func() { + factory := newFactoryOK(ptestNewConf, ptestNewErrType(), errToGetMaybe(ptestConfigurationFailedErr)) + f, ok := factory.(func() (ptestPlugin, error)) + Expect(ok).To(BeTrue()) + plugin, err := f() + Expect(err).To(Equal(ptestConfigurationFailedErr)) + Expect(plugin).To(BeNil()) + }) + + It("no err, get config failed, throw panic", func() { + factory := newFactoryOK(ptestNewConf, ptestNewType(), errToGetMaybe(ptestConfigurationFailedErr)) + f, ok := factory.(func() ptestPlugin) + Expect(ok).To(BeTrue()) + func() { + defer func() { + r := recover() + Expect(r).To(Equal(ptestConfigurationFailedErr)) + }() + f() + }() + }) + + It("panic on trim non nil err", func() { + factory := newFactoryOK(ptestNewErrFailing, ptestNewType(), nil) + f, ok := factory.(func() ptestPlugin) + Expect(ok).To(BeTrue()) + func() { + defer func() { + r := recover() + Expect(r).To(Equal(ptestCreateFailedErr)) + }() + f() + }() + }) - It("new factory add err", func() { - testee := newPluginConstructor(ptestType(), ptestNew) - factory, err := testee.NewFactory(ptestFactoryType(), nil) - Expect(err).NotTo(HaveOccurred()) - f, ok := factory.(func() (ptestPlugin, error)) - Expect(ok).To(BeTrue()) - plugin, err := f() - Expect(err).NotTo(HaveOccurred()) - ptestExpectConfigValue(plugin, ptestInitValue) }) +}) - It("new factory trim nil err", func() { - testee := newPluginConstructor(ptestType(), ptestNewImplErr) - factory, err := testee.NewFactory(ptestFactoryNoErrType(), nil) - Expect(err).NotTo(HaveOccurred()) - f, ok := factory.(func() ptestPlugin) - Expect(ok).To(BeTrue()) - plugin := f() - ptestExpectConfigValue(plugin, ptestInitValue) - }) +var _ = Describe("factory constructor", func() { + DescribeTable("expectations failed", + func(newPlugin interface{}) { + defer recoverExpectationFail() + newFactoryConstructor(ptestType(), newPlugin) + }, + Entry("not func", + errors.New("that is not constructor")), + Entry("returned not func", + func() error { panic("") }), + Entry("too many args", + func(_, _ ptestConfig) func() ptestPlugin { panic("") }), + Entry("too many return valued", + func() (func() ptestPlugin, error, error) { panic("") }), + Entry("second return value is not error", + func() (func() ptestPlugin, ptestPlugin) { panic("") }), + Entry("factory accepts conf", + func() func(config ptestConfig) ptestPlugin { panic("") }), + Entry("not implements", + func() func() struct{} { panic("") }), + Entry("factory too many args", + func() func(_, _ ptestConfig) ptestPlugin { panic("") }), + Entry("factory too many return valued", + func() func() (_ ptestPlugin, _, _ error) { panic("") }), + Entry("factory second return value is not error", + func() func() (_, _ ptestPlugin) { panic("") }), + ) - It("new factory config", func() { - testee := newPluginConstructor(ptestType(), ptestNewImplConf) - factory, err := testee.NewFactory(ptestFactoryNoErrType(), confToGetMaybe(ptestDefaultConf())) - Expect(err).NotTo(HaveOccurred()) - f, ok := factory.(func() ptestPlugin) - Expect(ok).To(BeTrue()) - plugin := f() - ptestExpectConfigValue(plugin, ptestDefaultValue) - }) + // FIXME - It("new factory, get config failed", func() { - testee := newPluginConstructor(ptestType(), ptestNewImplConf) - factory, err := testee.NewFactory(ptestFactoryType(), errToGetMaybe(ptestConfigurationFailedErr)) - Expect(err).NotTo(HaveOccurred()) - f, ok := factory.(func() (ptestPlugin, error)) - Expect(ok).To(BeTrue()) - plugin, err := f() - Expect(err).To(Equal(ptestConfigurationFailedErr)) - Expect(plugin).To(BeNil()) - }) +}) - It("new factory no err, get config failed, throw panic", func() { - testee := newPluginConstructor(ptestType(), ptestNewImplConf) - factory, err := testee.NewFactory(ptestFactoryNoErrType(), errToGetMaybe(ptestConfigurationFailedErr)) - Expect(err).NotTo(HaveOccurred()) - f, ok := factory.(func() ptestPlugin) - Expect(ok).To(BeTrue()) - func() { - defer func() { - r := recover() - Expect(r).To(Equal(ptestConfigurationFailedErr)) - }() - f() - }() - }) +func confToMaybe(conf interface{}) []reflect.Value { + if conf != nil { + return []reflect.Value{reflect.ValueOf(conf)} + } + return nil +} - It("new factory panic on trim non nil err", func() { - testee := newPluginConstructor(ptestType(), ptestNewImplErrFailed) - factory, err := testee.NewFactory(ptestFactoryNoErrType(), nil) - Expect(err).NotTo(HaveOccurred()) - f, ok := factory.(func() ptestPlugin) - Expect(ok).To(BeTrue()) - func() { - defer func() { - r := recover() - Expect(r).To(Equal(ptestCreateFailedErr)) - }() - f() - }() - }) +func confToGetMaybe(conf interface{}) func() ([]reflect.Value, error) { + return func() ([]reflect.Value, error) { + return confToMaybe(conf), nil + } +} -}) +func errToGetMaybe(err error) func() ([]reflect.Value, error) { + return func() ([]reflect.Value, error) { + return nil, err + } +} func expectSameFunc(f1, f2 interface{}) { s1 := fmt.Sprint(f1) diff --git a/core/plugin/plugin_test.go b/core/plugin/plugin_test.go index f59fa82e2..a3c9e37b5 100644 --- a/core/plugin/plugin_test.go +++ b/core/plugin/plugin_test.go @@ -21,7 +21,7 @@ var _ = Describe("Default registry", func() { Expect(Lookup(ptestType())).To(BeTrue()) }) It("lookup factory", func() { - Expect(LookupFactory(ptestFactoryType())).To(BeTrue()) + Expect(LookupFactory(ptestNewErrType())).To(BeTrue()) }) It("new", func() { plugin, err := New(ptestType(), ptestPluginName) @@ -29,7 +29,7 @@ var _ = Describe("Default registry", func() { Expect(plugin).NotTo(BeNil()) }) It("new factory", func() { - pluginFactory, err := NewFactory(ptestFactoryType(), ptestPluginName) + pluginFactory, err := NewFactory(ptestNewErrType(), ptestPluginName) Expect(err).NotTo(HaveOccurred()) Expect(pluginFactory).NotTo(BeNil()) }) @@ -41,7 +41,7 @@ var _ = Describe("type helpers", func() { Expect(PtrType(&plugin)).To(Equal(ptestType())) }) It("factory plugin type ok", func() { - factoryPlugin, ok := FactoryPluginType(ptestFactoryType()) + factoryPlugin, ok := FactoryPluginType(ptestNewErrType()) Expect(ok).To(BeTrue()) Expect(factoryPlugin).To(Equal(ptestType())) }) diff --git a/core/plugin/ptest_test.go b/core/plugin/ptest_test.go index 1dfd08a28..12064df0c 100644 --- a/core/plugin/ptest_test.go +++ b/core/plugin/ptest_test.go @@ -27,13 +27,11 @@ const ( func (r *Registry) ptestRegister(newPluginImpl interface{}, newDefaultConfigOptional ...interface{}) { r.Register(ptestType(), ptestPluginName, newPluginImpl, newDefaultConfigOptional...) } - func (r *Registry) ptestNew(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { return r.New(ptestType(), ptestPluginName, fillConfOptional...) } - func (r *Registry) ptestNewFactory(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { - factory, err := r.NewFactory(ptestFactoryType(), ptestPluginName, fillConfOptional...) + factory, err := r.NewFactory(ptestNewErrType(), ptestPluginName, fillConfOptional...) if err != nil { return } @@ -41,52 +39,50 @@ func (r *Registry) ptestNewFactory(fillConfOptional ...func(conf interface{}) er return typedFactory() } +var ( + ptestCreateFailedErr = errors.New("test plugin create failed") + ptestConfigurationFailedErr = errors.New("test plugin configuration failed") +) + type ptestPlugin interface { DoSomething() } - -func ptestType() reflect.Type { return reflect.TypeOf((*ptestPlugin)(nil)).Elem() } -func ptestImplType() reflect.Type { return reflect.TypeOf((*ptestImpl)(nil)).Elem() } - -func ptestFactoryType() reflect.Type { - return reflect.TypeOf(func() (ptestPlugin, error) { panic("") }) -} -func ptestFactoryNoErrType() reflect.Type { - return reflect.TypeOf(func() ptestPlugin { panic("") }) -} -func ptestFactoryConfigType() reflect.Type { - return reflect.TypeOf(func(ptestConfig) (ptestPlugin, error) { panic("") }) -} - type ptestImpl struct{ Value string } - -func (p *ptestImpl) DoSomething() {} - -var _ ptestPlugin = (*ptestImpl)(nil) - type ptestConfig struct{ Value string } -func ptestNew() ptestPlugin { return ptestNewImpl() } -func ptestNewImpl() *ptestImpl { return &ptestImpl{Value: ptestInitValue} } -func ptestNewImplConf(c ptestConfig) *ptestImpl { return &ptestImpl{c.Value} } -func ptestNewImplPtrConf(c *ptestConfig) *ptestImpl { - return &ptestImpl{c.Value} -} +func (p *ptestImpl) DoSomething() {} -func ptestNewImplErr() (*ptestImpl, error) { - return &ptestImpl{Value: ptestInitValue}, nil +func ptestNew() ptestPlugin { return ptestNewImpl() } +func ptestNewImpl() *ptestImpl { return &ptestImpl{Value: ptestInitValue} } +func ptestNewConf(c ptestConfig) ptestPlugin { return &ptestImpl{c.Value} } +func ptestNewPtrConf(c *ptestConfig) ptestPlugin { return &ptestImpl{c.Value} } +func ptestNewErr() (ptestPlugin, error) { return &ptestImpl{Value: ptestInitValue}, nil } +func ptestNewErrFailing() (ptestPlugin, error) { return nil, ptestCreateFailedErr } + +func ptestNewFactory() func() ptestPlugin { return ptestNew } +func ptestNewFactoryImpl() func() *ptestImpl { return ptestNewImpl } +func ptestNewFactoryConf() func(ptestConfig) ptestPlugin { + return func(c ptestConfig) ptestPlugin { + return ptestNewConf(c) + } } - -var ptestCreateFailedErr = errors.New("test plugin create failed") -var ptestConfigurationFailedErr = errors.New("test plugin configuration failed") - -func ptestNewImplErrFailed() (*ptestImpl, error) { - return nil, ptestCreateFailedErr +func ptestNewFactoryPtrConf() func(*ptestConfig) ptestPlugin { + return func(c *ptestConfig) ptestPlugin { + return ptestNewPtrConf(c) + } } +func ptestNewFactoryErr() (func() ptestPlugin, error) { return ptestNew, nil } +func ptestNewFactoryErrFailing() (func() ptestPlugin, error) { return nil, ptestCreateFailedErr } +func ptestNewFactoryFactoryErr() func() (ptestPlugin, error) { return ptestNewErr } +func ptestNewFactoryFactoryErrFailing() func() (ptestPlugin, error) { return ptestNewErrFailing } func ptestDefaultConf() ptestConfig { return ptestConfig{ptestDefaultValue} } func ptestNewDefaultPtrConf() *ptestConfig { return &ptestConfig{ptestDefaultValue} } +func ptestType() reflect.Type { return PtrType((*ptestPlugin)(nil)) } +func ptestNewErrType() reflect.Type { return reflect.TypeOf(ptestNewErr) } +func ptestNewType() reflect.Type { return reflect.TypeOf(ptestNew) } + func ptestFillConf(conf interface{}) error { return config.Decode(map[string]interface{}{"Value": ptestFilledValue}, conf) } diff --git a/core/plugin/registry_test.go b/core/plugin/registry_test.go index 8ba3e48d3..cfc7a9766 100644 --- a/core/plugin/registry_test.go +++ b/core/plugin/registry_test.go @@ -47,17 +47,17 @@ var _ = Describe("new default config container", func() { ptestExpectConfigValue(conf[0].Interface(), ptestFilledValue) }, Entry("no default config", - ptestNewImplConf), + ptestNewConf), Entry("no default ptr config", - ptestNewImplPtrConf), + ptestNewPtrConf), Entry("default config", - ptestNewImplConf, ptestDefaultConf), + ptestNewConf, ptestDefaultConf), Entry("default ptr config", - ptestNewImplPtrConf, ptestNewDefaultPtrConf), + ptestNewPtrConf, ptestNewDefaultPtrConf), ) It("fill no config failed", func() { - container := newDefaultConfigContainer(ptestFactoryType(), nil) + container := newDefaultConfigContainer(ptestNewErrType(), nil) _, err := container.Get(ptestFillConf) Expect(err).To(HaveOccurred()) }) @@ -182,35 +182,35 @@ var _ = Describe("new", func() { Expect(testNewOk()).To(Equal("")) }) It("default", func() { - r.ptestRegister(ptestNewImplConf, ptestDefaultConf) + r.ptestRegister(ptestNewConf, ptestDefaultConf) Expect(testNewOk()).To(Equal(ptestDefaultValue)) }) It("fill conf default", func() { - r.ptestRegister(ptestNewImplConf, ptestDefaultConf) + r.ptestRegister(ptestNewConf, ptestDefaultConf) Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) }) It("fill conf no default", func() { - r.ptestRegister(ptestNewImplConf) + r.ptestRegister(ptestNewConf) Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) }) It("fill ptr conf no default", func() { - r.ptestRegister(ptestNewImplPtrConf) + r.ptestRegister(ptestNewPtrConf) Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) }) It("no default ptr conf not nil", func() { - r.ptestRegister(ptestNewImplPtrConf) + r.ptestRegister(ptestNewPtrConf) Expect("").To(Equal(testNewOk())) }) It("nil default, conf not nil", func() { - r.ptestRegister(ptestNewImplPtrConf, func() *ptestConfig { return nil }) + r.ptestRegister(ptestNewPtrConf, func() *ptestConfig { return nil }) Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) }) It("fill nil default", func() { - r.ptestRegister(ptestNewImplPtrConf, func() *ptestConfig { return nil }) + r.ptestRegister(ptestNewPtrConf, func() *ptestConfig { return nil }) Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) }) It("more than one fill conf panics", func() { - r.ptestRegister(ptestNewImplPtrConf) + r.ptestRegister(ptestNewPtrConf) defer recoverExpectationFail() testNew(r, ptestFillConf, ptestFillConf) }) @@ -260,7 +260,7 @@ var _ = Describe("decode", func() { }) }) - r.Register(ptestType(), "my-plugin", ptestNewImplConf, ptestDefaultConf) + r.Register(ptestType(), "my-plugin", ptestNewConf, ptestDefaultConf) input := map[string]interface{}{ "plugin": map[string]interface{}{ nameKey: "my-plugin", From 80849dd65fb19e18e41a8964e96cbbe48df6583f Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Sat, 30 Sep 2017 22:53:09 +0300 Subject: [PATCH 10/15] Plugin factory constructor --- core/plugin/constructor.go | 2 - core/plugin/constructor_test.go | 138 +++++++++++++++++++++++++++++++- core/plugin/ptest_test.go | 4 +- core/plugin/registry.go | 15 +++- glide.lock | 40 ++++----- 5 files changed, 168 insertions(+), 31 deletions(-) diff --git a/core/plugin/constructor.go b/core/plugin/constructor.go index 734503d62..ad41606ca 100644 --- a/core/plugin/constructor.go +++ b/core/plugin/constructor.go @@ -48,8 +48,6 @@ func (c *pluginConstructor) NewPlugin(maybeConf []reflect.Value) (plugin interfa } func (c *pluginConstructor) NewFactory(factoryType reflect.Type, getMaybeConf func() ([]reflect.Value, error)) (interface{}, error) { - // FIXME: TEST no config and same type return newPlugin interface - // FIXME: TEST no error returning factories if c.newPlugin.Type() == factoryType { return c.newPlugin.Interface(), nil } diff --git a/core/plugin/constructor_test.go b/core/plugin/constructor_test.go index 83a8f2e10..8fdc62d6e 100644 --- a/core/plugin/constructor_test.go +++ b/core/plugin/constructor_test.go @@ -6,13 +6,13 @@ package plugin import ( + "errors" "fmt" "reflect" . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" - "github.com/pkg/errors" ) var _ = Describe("plugin constructor", func() { @@ -66,7 +66,7 @@ var _ = Describe("plugin constructor", func() { return factory } - It("from conversion", func() { + It("same type - no wrap", func() { factory := newFactoryOK(ptestNew, ptestNewType(), nil) expectSameFunc(factory, ptestNew) }) @@ -170,8 +170,140 @@ var _ = Describe("factory constructor", func() { func() func() (_, _ ptestPlugin) { panic("") }), ) - // FIXME + Context("new plugin", func() { + newPlugin := func(newFactory interface{}, maybeConf []reflect.Value) (interface{}, error) { + testee := newFactoryConstructor(ptestType(), newFactory) + return testee.NewPlugin(maybeConf) + } + + It("", func() { + plugin, err := newPlugin(ptestNewFactory, nil) + Expect(err).NotTo(HaveOccurred()) + ptestExpectConfigValue(plugin, ptestInitValue) + }) + + It("impl", func() { + plugin, err := newPlugin(ptestNewFactoryImpl, nil) + Expect(err).NotTo(HaveOccurred()) + ptestExpectConfigValue(plugin, ptestInitValue) + }) + + It("config", func() { + plugin, err := newPlugin(ptestNewFactoryConf, confToMaybe(ptestDefaultConf())) + Expect(err).NotTo(HaveOccurred()) + ptestExpectConfigValue(plugin, ptestDefaultValue) + }) + + It("failed", func() { + plugin, err := newPlugin(ptestNewFactoryErrFailing, nil) + Expect(err).To(Equal(ptestCreateFailedErr)) + Expect(plugin).To(BeNil()) + }) + + It("factory failed", func() { + plugin, err := newPlugin(ptestNewFactoryFactoryErrFailing, nil) + Expect(err).To(Equal(ptestCreateFailedErr)) + Expect(plugin).To(BeNil()) + }) + }) + + Context("new factory", func() { + newFactory := func(newFactory interface{}, factoryType reflect.Type, getMaybeConf func() ([]reflect.Value, error)) (interface{}, error) { + testee := newFactoryConstructor(ptestType(), newFactory) + return testee.NewFactory(factoryType, getMaybeConf) + } + newFactoryOK := func(newF interface{}, factoryType reflect.Type, getMaybeConf func() ([]reflect.Value, error)) interface{} { + factory, err := newFactory(newF, factoryType, getMaybeConf) + Expect(err).NotTo(HaveOccurred()) + return factory + } + + It("no err, same type - no wrap", func() { + factory := newFactoryOK(ptestNewFactory, ptestNewType(), nil) + expectSameFunc(factory, ptestNew) + }) + + It("has err, same type - no wrap", func() { + factory := newFactoryOK(ptestNewFactoryFactoryErr, ptestNewErrType(), nil) + expectSameFunc(factory, ptestNewErr) + }) + It("from new impl", func() { + factory := newFactoryOK(ptestNewFactoryImpl, ptestNewType(), nil) + f, ok := factory.(func() ptestPlugin) + Expect(ok).To(BeTrue()) + plugin := f() + ptestExpectConfigValue(plugin, ptestInitValue) + }) + + It("add err", func() { + factory := newFactoryOK(ptestNewFactory, ptestNewErrType(), nil) + f, ok := factory.(func() (ptestPlugin, error)) + Expect(ok).To(BeTrue()) + plugin, err := f() + Expect(err).NotTo(HaveOccurred()) + ptestExpectConfigValue(plugin, ptestInitValue) + }) + + It("factory construction not failed", func() { + factory := newFactoryOK(ptestNewFactoryErr, ptestNewType(), nil) + f, ok := factory.(func() ptestPlugin) + Expect(ok).To(BeTrue()) + plugin := f() + ptestExpectConfigValue(plugin, ptestInitValue) + }) + + It("trim nil err", func() { + factory := newFactoryOK(ptestNewFactoryFactoryErr, ptestNewType(), nil) + f, ok := factory.(func() ptestPlugin) + Expect(ok).To(BeTrue()) + plugin := f() + ptestExpectConfigValue(plugin, ptestInitValue) + }) + + It("config", func() { + factory := newFactoryOK(ptestNewFactoryConf, ptestNewType(), confToGetMaybe(ptestDefaultConf())) + f, ok := factory.(func() ptestPlugin) + Expect(ok).To(BeTrue()) + plugin := f() + ptestExpectConfigValue(plugin, ptestDefaultValue) + }) + + It("get config failed", func() { + factory, err := newFactory(ptestNewFactoryConf, ptestNewErrType(), errToGetMaybe(ptestConfigurationFailedErr)) + Expect(err).To(Equal(ptestConfigurationFailedErr)) + Expect(factory).To(BeNil()) + }) + + It("factory create failed", func() { + factory, err := newFactory(ptestNewFactoryErrFailing, ptestNewErrType(), nil) + Expect(err).To(Equal(ptestCreateFailedErr)) + Expect(factory).To(BeNil()) + }) + + It("plugin create failed", func() { + factory := newFactoryOK(ptestNewFactoryFactoryErrFailing, ptestNewErrType(), nil) + f, ok := factory.(func() (ptestPlugin, error)) + Expect(ok).To(BeTrue()) + plugin, err := f() + Expect(err).To(Equal(ptestCreateFailedErr)) + Expect(plugin).To(BeNil()) + }) + + It("panic on trim non nil err", func() { + factory := newFactoryOK(ptestNewFactoryFactoryErrFailing, ptestNewType(), nil) + f, ok := factory.(func() ptestPlugin) + Expect(ok).To(BeTrue()) + func() { + defer func() { + r := recover() + Expect(r).To(Equal(ptestCreateFailedErr)) + }() + f() + }() + }) + + }) }) func confToMaybe(conf interface{}) []reflect.Value { diff --git a/core/plugin/ptest_test.go b/core/plugin/ptest_test.go index 12064df0c..9cbe58a0d 100644 --- a/core/plugin/ptest_test.go +++ b/core/plugin/ptest_test.go @@ -61,8 +61,8 @@ func ptestNewErrFailing() (ptestPlugin, error) { return nil, ptestCreateFailed func ptestNewFactory() func() ptestPlugin { return ptestNew } func ptestNewFactoryImpl() func() *ptestImpl { return ptestNewImpl } -func ptestNewFactoryConf() func(ptestConfig) ptestPlugin { - return func(c ptestConfig) ptestPlugin { +func ptestNewFactoryConf(c ptestConfig) func() ptestPlugin { + return func() ptestPlugin { return ptestNewConf(c) } } diff --git a/core/plugin/registry.go b/core/plugin/registry.go index 5cff28490..28a13964d 100644 --- a/core/plugin/registry.go +++ b/core/plugin/registry.go @@ -80,9 +80,17 @@ func (r *Registry) NewFactory(factoryType reflect.Type, name string, fillConfOpt if err != nil { return } - // OPTIMIZE: not create getMaybeConfig when there no config required. Just check it once on empty struct ptr. - getMaybeConfig := func() ([]reflect.Value, error) { - return registered.defaultConfig.Get(fillConf) + var getMaybeConfig func() ([]reflect.Value, error) + if registered.defaultConfig.configRequired() { + getMaybeConfig = func() ([]reflect.Value, error) { + return registered.defaultConfig.Get(fillConf) + } + } else if fillConf != nil { + // Just check, that fillConf not fails, when there is no config fields. + err := fillConf(&struct{}{}) + if err != nil { + return nil, err + } } return registered.constructor.NewFactory(factoryType, getMaybeConfig) } @@ -114,7 +122,6 @@ type defaultConfigContainer struct { newValue reflect.Value } -// newDefaultConfigContainer should not be called func newDefaultConfigContainer(constructorType reflect.Type, newDefaultConfig interface{}) defaultConfigContainer { if constructorType.NumIn() == 0 { expect(newDefaultConfig == nil, "constructor accept no config, but newDefaultConfig passed") diff --git a/glide.lock b/glide.lock index 9bf34c575..0735d8eef 100644 --- a/glide.lock +++ b/glide.lock @@ -1,10 +1,10 @@ -hash: b06d35c760b8a60202cc52695b1dd3d3ac96acdbbb9a8cd52c0cba9a05880733 -updated: 2017-08-25T10:25:48.281437192+03:00 +hash: e9c64f4ab9d84f20f3ea357eb84c21becc99b134765591aec67efddbae7d46f0 +updated: 2017-09-30T22:54:19.407523708+03:00 imports: - name: github.com/amahi/spdy version: 31da8b754faf6833fa95192c69bdb10518e9fb7b - name: github.com/asaskevich/govalidator - version: 15028e809df8c71964e8efa6c11e81d5c0262302 + version: 6fcd5b427f532a5d13738b27415e00a49e36ceef - name: github.com/c2h5oh/datasize version: 54516c931ae99c3c74637b9ea2390cf9a6327f26 - name: github.com/davecgh/go-spew @@ -20,7 +20,7 @@ imports: - name: github.com/fsnotify/fsnotify version: 4da3e2cfbabc9f751898f250b49f2439785783a1 - name: github.com/hashicorp/hcl - version: 392dba7d905ed5d04a5794ba89f558b27e2ba1ca + version: 68e816d1c783414e79bc65b3994d9ab6b0a722ab subpackages: - hcl/ast - hcl/parser @@ -31,12 +31,12 @@ imports: - json/scanner - json/token - name: github.com/magiconair/properties - version: be5ece7dd465ab0765a9682137865547526d1dfb + version: 8d7837e64d3c1ee4e54a880c5a920ab4316fc90a - name: github.com/mitchellh/mapstructure version: e88fb6b4946b282e0d5196ac35b09d256e09e9d2 repo: https://github.com/skipor/mapstructure - name: github.com/onsi/ginkgo - version: 8382b23d18dbaaff8e5f7e83784c53ebb8ec2f47 + version: 11459a886d9cd66b319dac7ef1e917ee221372c9 subpackages: - config - extensions/table @@ -57,7 +57,7 @@ imports: - reporters/stenographer/support/go-isatty - types - name: github.com/onsi/gomega - version: c893efa28eb45626cdaa76c9f653b62488858837 + version: dcabb60a477c2b6f456df65037cb6708210fbb02 subpackages: - format - gbytes @@ -75,30 +75,30 @@ imports: - matchers/support/goraph/util - types - name: github.com/pelletier/go-toml - version: 9c1b4e331f1e3d98e72600677699fbe212cd6d16 + version: 16398bac157da96aa88f98a2df640c7f32af1da2 - name: github.com/pkg/errors - version: c605e284fe17294bda444b34710735b29d1a9d90 + version: 2b3a18b5f0fb6b4f9190549597d3f962c02bc5eb - name: github.com/pmezard/go-difflib version: d8ed2627bdf02c080bf22230dbb337003b7aba2d subpackages: - difflib - name: github.com/pquerna/ffjson - version: 9a5203b7a07166f217f5f8177d5b16177acad3b2 + version: 619064c2092f8ed31957bf7af846f1af0529b737 subpackages: - fflib/v1 - fflib/v1/internal - name: github.com/spf13/afero - version: 9be650865eab0c12963d8753212f4f9c66cdcf12 + version: ee1bd8ee15a1306d1f9201acc41ef39cd9f99a1b subpackages: - mem - name: github.com/spf13/cast version: acbeb36b902d72a7a4c18e8f3241075e7ab763e4 - name: github.com/spf13/jwalterweatherman - version: 0efa5202c04663c757d84f90f5219c1250baf94f + version: 12bd96e66386c1960ab0f74ced1362f66f552f7b - name: github.com/spf13/pflag - version: e57e3eeb33f795204c1ca35f56c44f83227c6e66 + version: 7aff26db30c1be810f9de5038ec5ef96ac41fd7c - name: github.com/spf13/viper - version: 25b30aa063fc18e48662b86996252eabdcf2f0c7 + version: d9cca5ef33035202efb1586825bdbb15ff9ec3ba - name: github.com/stretchr/objx version: cbeaeb16a013161a98496fad62933b1d21786672 - name: github.com/stretchr/testify @@ -108,13 +108,13 @@ imports: - mock - require - name: github.com/uber-go/atomic - version: 70bd1261d36be490ebd22a62b385a3c5d23b6240 + version: 54f72d32435d760d5604f17a82e2435b28dc4ba5 - name: go.uber.org/atomic - version: 70bd1261d36be490ebd22a62b385a3c5d23b6240 + version: 4e336646b2ef9fc6e47be8e21594178f98e5ebcf - name: go.uber.org/multierr version: 3c4937480c32f4c13a875a1829af76c98ca3d40a - name: go.uber.org/zap - version: 416e66ad83ebde35df0e09f02e65ac149e193b0e + version: 35aad584952c3e7020db7b839f6b102de6271f89 subpackages: - buffer - internal/bufferpool @@ -122,7 +122,7 @@ imports: - internal/exit - zapcore - name: golang.org/x/net - version: 57efc9c3d9f91fb3277f8da1cff370539c4d3dc5 + version: 0a9397675ba34b2845f758fe3cd68828369c6517 subpackages: - html - html/atom @@ -132,11 +132,11 @@ imports: - idna - lex/httplex - name: golang.org/x/sys - version: 07c182904dbd53199946ba614a412c61d3c548f5 + version: 314a259e304ff91bd6985da2a7149bbf91237993 subpackages: - unix - name: golang.org/x/text - version: ac87088df8ef557f1e32cd00ed0b6fbc3f7ddafb + version: 1cbadb444a806fd9430d14ad08967ed91da4fa0a subpackages: - encoding - encoding/charmap From 39acf3f4f7ae6c7c4ca8f9f748633459b36a577e Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Sun, 1 Oct 2017 12:06:22 +0300 Subject: [PATCH 11/15] Update plugin package docs --- core/plugin/constructor.go | 38 +++++++++++++++---------- core/plugin/constructor_test.go | 30 +++++++++++++++++++- core/plugin/doc.go | 29 +++++++++---------- core/plugin/plugin.go | 30 ++++---------------- core/plugin/ptest_test.go | 19 +++++++++---- core/plugin/registry.go | 50 +++++++++++++++++++++++++++------ 6 files changed, 125 insertions(+), 71 deletions(-) diff --git a/core/plugin/constructor.go b/core/plugin/constructor.go index ad41606ca..399befd7d 100644 --- a/core/plugin/constructor.go +++ b/core/plugin/constructor.go @@ -10,12 +10,12 @@ import ( "reflect" ) -// constructor interface representing ability to create some plugin interface +// implConstructor interface representing ability to create some plugin interface // implementations and is't factory functions. Usually it wraps some newPlugin or newFactory function, and // use is as implementation creator. -// constructor expects, that caller pass correct maybeConf value, that can be +// implConstructor expects, that caller pass correct maybeConf value, that can be // passed to underlying implementation creator. -type constructor interface { +type implConstructor interface { // NewPlugin constructs plugin implementation. NewPlugin(maybeConf []reflect.Value) (plugin interface{}, err error) // getMaybeConf may be nil, if no config required. @@ -24,18 +24,27 @@ type constructor interface { NewFactory(factoryType reflect.Type, getMaybeConf func() ([]reflect.Value, error)) (pluginFactory interface{}, err error) } +func newImplConstructor(pluginType reflect.Type, constructor interface{}) implConstructor { + constructorType := reflect.TypeOf(constructor) + expect(constructorType.Kind() == reflect.Func, "plugin constructor should be func") + expect(constructorType.NumOut() >= 1, + "plugin constructor should return plugin implementation as first output parameter") + if constructorType.Out(0).Kind() == reflect.Func { + return newFactoryConstructor(pluginType, constructor) + } + return newPluginConstructor(pluginType, constructor) +} + func newPluginConstructor(pluginType reflect.Type, newPlugin interface{}) *pluginConstructor { expectPluginConstructor(pluginType, reflect.TypeOf(newPlugin), true) return &pluginConstructor{pluginType, reflect.ValueOf(newPlugin)} } -// pluginConstructor is abstract constructor of some pluginType interface implementations -// and it's factory functions, using some func newPlugin func. +// pluginConstructor use newPlugin func([config ]) ( [, error]) +// to construct plugin implementations type pluginConstructor struct { pluginType reflect.Type - // newPlugin type is func([config ]) ( [, error]), - // where configType kind is struct or struct pointer. - newPlugin reflect.Value + newPlugin reflect.Value } func (c *pluginConstructor) NewPlugin(maybeConf []reflect.Value) (plugin interface{}, err error) { @@ -68,15 +77,14 @@ func (c *pluginConstructor) NewFactory(factoryType reflect.Type, getMaybeConf fu } } out := c.newPlugin.Call(maybeConf) - return convertFactoryOutParam(c.pluginType, factoryType.NumOut(), out) + return convertFactoryOutParams(c.pluginType, factoryType.NumOut(), out) }).Interface(), nil } -// factoryConstructor is abstract constructor of some pluginType interface and it's -// factory functions, using some func newFactory func. +// factoryConstructor use newFactory func([config ]) (func() ([, error])[, error) +// to construct plugin implementations. type factoryConstructor struct { pluginType reflect.Type - // newFactory type is func([config ]) (func() ([, error])[, error), newFactory reflect.Value } @@ -126,7 +134,7 @@ func (c *factoryConstructor) NewFactory(factoryType reflect.Type, getMaybeConf f } return reflect.MakeFunc(factoryType, func(in []reflect.Value) []reflect.Value { out := factory.Call(nil) - return convertFactoryOutParam(c.pluginType, factoryType.NumOut(), out) + return convertFactoryOutParams(c.pluginType, factoryType.NumOut(), out) }).Interface(), nil } @@ -155,8 +163,8 @@ func expectPluginConstructor(pluginType, factoryType reflect.Type, configAllowed } } -// convertFactoryOutParam converts output params of some factory (newFactory) call, to required. -func convertFactoryOutParam(pluginType reflect.Type, numOut int, out []reflect.Value) []reflect.Value { +// convertFactoryOutParams converts output params of some factory (newPlugin) call to required. +func convertFactoryOutParams(pluginType reflect.Type, numOut int, out []reflect.Value) []reflect.Value { switch numOut { case 1, 2: // OK. diff --git a/core/plugin/constructor_test.go b/core/plugin/constructor_test.go index 8fdc62d6e..f51b94401 100644 --- a/core/plugin/constructor_test.go +++ b/core/plugin/constructor_test.go @@ -45,6 +45,12 @@ var _ = Describe("plugin constructor", func() { ptestExpectConfigValue(plugin, ptestInitValue) }) + It("more that plugin", func() { + plugin, err := newPlugin(ptestNewMoreThan, nil) + Expect(err).NotTo(HaveOccurred()) + ptestExpectConfigValue(plugin, ptestInitValue) + }) + It("config", func() { plugin, err := newPlugin(ptestNewConf, confToMaybe(ptestDefaultConf())) Expect(err).NotTo(HaveOccurred()) @@ -71,7 +77,7 @@ var _ = Describe("plugin constructor", func() { expectSameFunc(factory, ptestNew) }) - It("from new impl", func() { + It(" new impl", func() { factory := newFactoryOK(ptestNewImpl, ptestNewType(), nil) f, ok := factory.(func() ptestPlugin) Expect(ok).To(BeTrue()) @@ -79,6 +85,14 @@ var _ = Describe("plugin constructor", func() { ptestExpectConfigValue(plugin, ptestInitValue) }) + It("more than", func() { + factory := newFactoryOK(ptestNewMoreThan, ptestNewType(), nil) + f, ok := factory.(func() ptestPlugin) + Expect(ok).To(BeTrue()) + plugin := f() + ptestExpectConfigValue(plugin, ptestInitValue) + }) + It("add err", func() { factory := newFactoryOK(ptestNew, ptestNewErrType(), nil) f, ok := factory.(func() (ptestPlugin, error)) @@ -188,6 +202,12 @@ var _ = Describe("factory constructor", func() { ptestExpectConfigValue(plugin, ptestInitValue) }) + It("impl more than", func() { + plugin, err := newPlugin(ptestNewFactoryMoreThan, nil) + Expect(err).NotTo(HaveOccurred()) + ptestExpectConfigValue(plugin, ptestInitValue) + }) + It("config", func() { plugin, err := newPlugin(ptestNewFactoryConf, confToMaybe(ptestDefaultConf())) Expect(err).NotTo(HaveOccurred()) @@ -236,6 +256,14 @@ var _ = Describe("factory constructor", func() { ptestExpectConfigValue(plugin, ptestInitValue) }) + It("from new impl", func() { + factory := newFactoryOK(ptestNewFactoryMoreThan, ptestNewType(), nil) + f, ok := factory.(func() ptestPlugin) + Expect(ok).To(BeTrue()) + plugin := f() + ptestExpectConfigValue(plugin, ptestInitValue) + }) + It("add err", func() { factory := newFactoryOK(ptestNewFactory, ptestNewErrType(), nil) f, ok := factory.(func() (ptestPlugin, error)) diff --git a/core/plugin/doc.go b/core/plugin/doc.go index e99b3f863..59b90d8df 100644 --- a/core/plugin/doc.go +++ b/core/plugin/doc.go @@ -7,27 +7,24 @@ // extensible Go packages, libraries, and applications. Like // github.com/progrium/go-extpoints, but reflect based: doesn't require code // generation, but have more overhead; provide more flexibility, but less type -// safety. It allows to register factory for some plugin interface, and create -// new plugin instances by registered factory. +// safety. It allows to register constructor for some plugin interface, and create +// new plugin instances or plugin instance factories. // Main feature is flexible plugin configuration: plugin factory can // accept config struct, that could be filled by passed hook. Config default // values could be provided by registering default config factory. // Such flexibility can be used to decode structured text (json/yaml/etc) into -// struct with plugin interface fields. +// struct. // // Type expectations. -// Plugin factory type should be: -// func([config ]) ([, error]) -// where configType kind is struct or struct pointer, and pluginImpl implements -// plugin interface. Plugin factory will never receive nil config, even there -// are no registered default config factory, or default config is nil. Config -// will be pointer to zero config in such case. -// If plugin factory receive config argument, default config factory can be -// registered. Default config factory type should be: is func() . -// Default config factory is optional. If no default config factory has been -// registered, than plugin factory will receive zero config (zero struct or -// pointer to zero struct). +// Here and bellow we mean by some type expectations. +// [some type signature part] means that this part of type signature is optional. // -// Note, that plugin interface type could be taken as reflect.TypeOf((*PluginInterface)(nil)).Elem(). -// FIXME: doc plugin factory +// Plugin type, let's label it as , should be interface. +// Registered plugin constructor should be one of: or . +// should have type func([config ]) ([, error]). +// should have type func([config [, error])[, error]). +// should be assignable to . +// That is, methods should be subset methods. In other words, should be +// some implementation, or interface, that contains methods as subset. +// type should be struct or struct pointer. package plugin diff --git a/core/plugin/plugin.go b/core/plugin/plugin.go index 24082cd5a..dcaa2ddfc 100644 --- a/core/plugin/plugin.go +++ b/core/plugin/plugin.go @@ -10,15 +10,12 @@ import ( "reflect" ) +// DefaultRegistry returns default Registry used for package Registry like functions. func DefaultRegistry() *Registry { return defaultRegistry } -// Register registers plugin factory and optional default config factory, -// for given plugin interface type and plugin name. -// See package doc for type expectations details. -// Register designed to be called in package init func, so it panics if type -// expectations were failed. Register is thread unsafe. +// Register is DefaultRegistry().Register shortcut. func Register( pluginType reflect.Type, name string, @@ -28,37 +25,22 @@ func Register( DefaultRegistry().Register(pluginType, name, newPluginImpl, newDefaultConfigOptional...) } -// Lookup returns true if any plugin factory has been registered for given -// type. +// Lookup is DefaultRegistry().Lookup shortcut. func Lookup(pluginType reflect.Type) bool { return DefaultRegistry().Lookup(pluginType) } -// LookupFactory returns true if factoryType looks like func() (SomeInterface[, error]) -// and any plugin factory has been registered for SomeInterface. +// LookupFactory is DefaultRegistry().LookupFactory shortcut. func LookupFactory(factoryType reflect.Type) bool { return DefaultRegistry().LookupFactory(factoryType) } -// New creates plugin by registered plugin factory. Returns error if creation -// failed or no plugin were registered for given type and name. -// Passed fillConf called on created config before calling plugin factory. -// fillConf argument is always valid struct pointer, even if plugin factory -// receives no config: fillConf is called on empty struct pointer in such case. -// fillConf error fails plugin creation. -// New is thread safe, if there is no concurrent Register calls. +// New is DefaultRegistry().New shortcut. func New(pluginType reflect.Type, name string, fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { return defaultRegistry.New(pluginType, name, fillConfOptional...) } -// TODO (skipor): add support for `func() PluginInterface` factories, that -// panics on error. -// TODO (skipor): add NewSharedConfigsFactory that decodes config once and use -// it to create all plugin instances. - -// NewFactory behaves like New, but creates factory func() (PluginInterface, error), that on call -// creates New plugin by registered factory. -// New config is created filled for every factory call. +// NewFactory is DefaultRegistry().NewFactory shortcut. func NewFactory(factoryType reflect.Type, name string, fillConfOptional ...func(conf interface{}) error) (factory interface{}, err error) { return defaultRegistry.NewFactory(factoryType, name, fillConfOptional...) } diff --git a/core/plugin/ptest_test.go b/core/plugin/ptest_test.go index 9cbe58a0d..84f05dfe9 100644 --- a/core/plugin/ptest_test.go +++ b/core/plugin/ptest_test.go @@ -14,7 +14,7 @@ import ( "github.com/yandex/pandora/core/config" ) -// ptest contains samples and utils for testing plugin pkg +// ptest contains examples and utils for testing plugin pkg const ( ptestPluginName = "ptest_name" @@ -47,27 +47,34 @@ var ( type ptestPlugin interface { DoSomething() } +type ptestMoreThanPlugin interface { + ptestPlugin + DoSomethingElse() +} type ptestImpl struct{ Value string } type ptestConfig struct{ Value string } -func (p *ptestImpl) DoSomething() {} +func (p *ptestImpl) DoSomething() {} +func (p *ptestImpl) DoSomethingElse() {} func ptestNew() ptestPlugin { return ptestNewImpl() } +func ptestNewMoreThan() ptestMoreThanPlugin { return ptestNewImpl() } func ptestNewImpl() *ptestImpl { return &ptestImpl{Value: ptestInitValue} } func ptestNewConf(c ptestConfig) ptestPlugin { return &ptestImpl{c.Value} } func ptestNewPtrConf(c *ptestConfig) ptestPlugin { return &ptestImpl{c.Value} } func ptestNewErr() (ptestPlugin, error) { return &ptestImpl{Value: ptestInitValue}, nil } func ptestNewErrFailing() (ptestPlugin, error) { return nil, ptestCreateFailedErr } -func ptestNewFactory() func() ptestPlugin { return ptestNew } -func ptestNewFactoryImpl() func() *ptestImpl { return ptestNewImpl } +func ptestNewFactory() func() ptestPlugin { return ptestNew } +func ptestNewFactoryMoreThan() func() ptestMoreThanPlugin { return ptestNewMoreThan } +func ptestNewFactoryImpl() func() *ptestImpl { return ptestNewImpl } func ptestNewFactoryConf(c ptestConfig) func() ptestPlugin { return func() ptestPlugin { return ptestNewConf(c) } } -func ptestNewFactoryPtrConf() func(*ptestConfig) ptestPlugin { - return func(c *ptestConfig) ptestPlugin { +func ptestNewFactoryPtrConf(c *ptestConfig) func() ptestPlugin { + return func() ptestPlugin { return ptestNewPtrConf(c) } } diff --git a/core/plugin/registry.go b/core/plugin/registry.go index 28a13964d..96c14022d 100644 --- a/core/plugin/registry.go +++ b/core/plugin/registry.go @@ -24,15 +24,31 @@ func newNameRegistry() nameRegistry { return make(nameRegistry) } type nameRegistry map[string]nameRegistryEntry type nameRegistryEntry struct { - constructor constructor + constructor implConstructor defaultConfig defaultConfigContainer } +// Register registers plugin constructor and optional default config factory, +// for given plugin interface type and plugin name. +// See package doc for type expectations details. +// Register designed to be called in package init func, so it panics if something go wrong. +// Panics if type expectations are violated. +// Panics if some constructor have been already registered for this (pluginType, name) pair. +// Register is thread unsafe. +// +// If constructor receive config argument, default config factory can be +// registered. Default config factory type should be: is func() . +// Default config factory is optional. If no default config factory has been +// registered, than plugin factory will receive zero config (zero struct or +// pointer to zero struct). +// Registered constructor will never receive nil config, even there +// are no registered default config factory, or default config is nil. Config +// will be pointer to zero config in such case. func (r *Registry) Register( - pluginType reflect.Type, // plugin interface type + pluginType reflect.Type, name string, - newPluginImpl interface{}, - newDefaultConfigOptional ...interface{}, + constructor interface{}, + newDefaultConfigOptional ...interface{}, // default config factory, or nothing. ) { expect(pluginType.Kind() == reflect.Interface, "plugin type should be interface, but have: %T", pluginType) expect(name != "", "empty name") @@ -44,18 +60,30 @@ func (r *Registry) Register( _, ok := nameReg[name] expect(!ok, "plugin %s with name %q had been already registered", pluginType, name) newDefaultConfig := getNewDefaultConfig(newDefaultConfigOptional) - nameReg[name] = newNameRegistryEntry(pluginType, newPluginImpl, newDefaultConfig) + nameReg[name] = newNameRegistryEntry(pluginType, constructor, newDefaultConfig) } +// Lookup returns true if any plugin constructor has been registered for given +// type. func (r *Registry) Lookup(pluginType reflect.Type) bool { _, ok := r.typeToNameReg[pluginType] return ok } +// LookupFactory returns true if factoryType looks like func() (SomeInterface[, error]) +// and any plugin constructor has been registered for SomeInterface. +// That is, you may create instance of this factoryType using this registry. func (r *Registry) LookupFactory(factoryType reflect.Type) bool { return isFactoryType(factoryType) && r.Lookup(factoryType.Out(0)) } +// New creates plugin using registered plugin constructor. Returns error if creation +// failed or no plugin were registered for given type and name. +// Passed fillConf called on created config before calling plugin factory. +// fillConf argument is always valid struct pointer, even if plugin factory +// receives no config: fillConf is called on empty struct pointer in such case. +// fillConf error fails plugin creation. +// New is thread safe, if there is no concurrent Register calls. func (r *Registry) New(pluginType reflect.Type, name string, fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { expect(pluginType.Kind() == reflect.Interface, "plugin type should be interface, but have: %T", pluginType) expect(name != "", "empty name") @@ -71,6 +99,10 @@ func (r *Registry) New(pluginType reflect.Type, name string, fillConfOptional .. return registered.constructor.NewPlugin(conf) } +// NewFactory behaves like New, but creates factory func() (PluginInterface[, error]), that on call +// creates New plugin by registered factory. +// If registered constructor is config is created filled for every factory call, +// if Date: Sun, 1 Oct 2017 13:05:08 +0300 Subject: [PATCH 12/15] Test plugin factory constructor --- core/plugin/ptest_test.go | 4 +- core/plugin/registry_test.go | 247 +++++++++++++++++++---------------- 2 files changed, 133 insertions(+), 118 deletions(-) diff --git a/core/plugin/ptest_test.go b/core/plugin/ptest_test.go index 84f05dfe9..cfae16f84 100644 --- a/core/plugin/ptest_test.go +++ b/core/plugin/ptest_test.go @@ -24,8 +24,8 @@ const ( ptestFilledValue = "ptest_FILLED" ) -func (r *Registry) ptestRegister(newPluginImpl interface{}, newDefaultConfigOptional ...interface{}) { - r.Register(ptestType(), ptestPluginName, newPluginImpl, newDefaultConfigOptional...) +func (r *Registry) ptestRegister(constructor interface{}, newDefaultConfigOptional ...interface{}) { + r.Register(ptestType(), ptestPluginName, constructor, newDefaultConfigOptional...) } func (r *Registry) ptestNew(fillConfOptional ...func(conf interface{}) error) (plugin interface{}, err error) { return r.New(ptestType(), ptestPluginName, fillConfOptional...) diff --git a/core/plugin/registry_test.go b/core/plugin/registry_test.go index cfc7a9766..5eb1da323 100644 --- a/core/plugin/registry_test.go +++ b/core/plugin/registry_test.go @@ -63,63 +63,6 @@ var _ = Describe("new default config container", func() { }) }) -var _ = DescribeTable("register valid", - func( - newPluginImpl interface{}, - newDefaultConfigOptional ...interface{}, - ) { - Expect(func() { - NewRegistry().ptestRegister(newPluginImpl, newDefaultConfigOptional...) - }).NotTo(Panic()) - }, - Entry("return impl", - func() *ptestImpl { return nil }), - Entry("return interface", - func() ptestPlugin { return nil }), - Entry("super interface", - func() interface { - io.Writer - ptestPlugin - } { - return nil - }), - Entry("struct config", - func(ptestConfig) ptestPlugin { return nil }), - Entry("struct ptr config", - func(*ptestConfig) ptestPlugin { return nil }), - Entry("default config", - func(*ptestConfig) ptestPlugin { return nil }, - func() *ptestConfig { return nil }), -) - -var _ = DescribeTable("register invalid", - func( - newPluginImpl interface{}, - newDefaultConfigOptional ...interface{}, - ) { - Expect(func() { - defer recoverExpectationFail() - NewRegistry().ptestRegister(newPluginImpl, newDefaultConfigOptional...) - }).NotTo(Panic()) - }, - Entry("return not impl", - func() ptestImpl { panic("") }), - Entry("invalid config type", - func(int) ptestPlugin { return nil }), - Entry("invalid config ptr type", - func(*int) ptestPlugin { return nil }), - Entry("to many args", - func(_, _ ptestConfig) ptestPlugin { return nil }), - Entry("default without config", - func() ptestPlugin { return nil }, func() *ptestConfig { return nil }), - Entry("extra default config", - func(*ptestConfig) ptestPlugin { return nil }, func() *ptestConfig { return nil }, 0), - Entry("invalid default config", - func(ptestConfig) ptestPlugin { return nil }, func() *ptestConfig { return nil }), - Entry("default config accepts args", - func(*ptestConfig) ptestPlugin { return nil }, func(int) *ptestConfig { return nil }), -) - var _ = Describe("registry", func() { It("register name collision panics", func() { r := NewRegistry() @@ -127,6 +70,7 @@ var _ = Describe("registry", func() { defer recoverExpectationFail() r.ptestRegister(ptestNewImpl) }) + It("lookup", func() { r := NewRegistry() r.ptestRegister(ptestNewImpl) @@ -136,6 +80,17 @@ var _ = Describe("registry", func() { Expect(r.Lookup(reflect.TypeOf((*io.Writer)(nil)).Elem())).To(BeFalse()) }) + It("lookup factory", func() { + r := NewRegistry() + r.ptestRegister(ptestNewImpl) + Expect(r.LookupFactory(ptestNewType())).To(BeTrue()) + Expect(r.LookupFactory(ptestNewErrType())).To(BeTrue()) + + Expect(r.LookupFactory(reflect.TypeOf(0))).To(BeFalse()) + Expect(r.LookupFactory(reflect.TypeOf(&ptestImpl{}))).To(BeFalse()) + Expect(r.LookupFactory(reflect.TypeOf((*io.Writer)(nil)).Elem())).To(BeFalse()) + }) + }) var _ = Describe("new", func() { @@ -151,68 +106,128 @@ var _ = Describe("new", func() { ) BeforeEach(func() { r = NewRegistry() }) runTestCases := func() { - It("no conf", func() { - r.ptestRegister(ptestNewImpl) - Expect(testNewOk()).To(Equal(ptestInitValue)) - }) - It("nil error", func() { - r.ptestRegister(func() (ptestPlugin, error) { - return ptestNewImpl(), nil + Context("plugin constructor", func() { + It("no conf", func() { + r.ptestRegister(ptestNewImpl) + Expect(testNewOk()).To(Equal(ptestInitValue)) }) - Expect(testNewOk()).To(Equal(ptestInitValue)) - }) - It("non-nil error", func() { - expectedErr := errors.New("fill conf err") - r.ptestRegister(func() (ptestPlugin, error) { - return nil, expectedErr + It("nil error", func() { + r.ptestRegister(ptestNewErr) + Expect(testNewOk()).To(Equal(ptestInitValue)) + }) + It("non-nil error", func() { + r.ptestRegister(ptestNewErrFailing) + _, err := testNew(r) + Expect(err).To(HaveOccurred()) + err = errors.Cause(err) + Expect(ptestCreateFailedErr).To(Equal(err)) + }) + It("no conf, fill conf error", func() { + r.ptestRegister(ptestNewImpl) + expectedErr := errors.New("fill conf err") + _, err := testNew(r, func(_ interface{}) error { return expectedErr }) + Expect(expectedErr).To(Equal(err)) + }) + It("no default", func() { + r.ptestRegister(ptestNewConf) + Expect(testNewOk()).To(Equal("")) + }) + It("default", func() { + r.ptestRegister(ptestNewConf, ptestDefaultConf) + Expect(testNewOk()).To(Equal(ptestDefaultValue)) + }) + It("fill conf default", func() { + r.ptestRegister(ptestNewConf, ptestDefaultConf) + Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) + }) + It("fill conf no default", func() { + r.ptestRegister(ptestNewConf) + Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) + }) + It("fill ptr conf no default", func() { + r.ptestRegister(ptestNewPtrConf) + Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) + }) + It("no default ptr conf not nil", func() { + r.ptestRegister(ptestNewPtrConf) + Expect("").To(Equal(testNewOk())) + }) + It("nil default, conf not nil", func() { + r.ptestRegister(ptestNewPtrConf, func() *ptestConfig { return nil }) + Expect(testNewOk()).To(Equal("")) + }) + It("fill nil default", func() { + r.ptestRegister(ptestNewPtrConf, func() *ptestConfig { return nil }) + Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) + }) + It("more than one fill conf panics", func() { + r.ptestRegister(ptestNewPtrConf) + defer recoverExpectationFail() + testNew(r, ptestFillConf, ptestFillConf) }) - _, err := testNew(r) - Expect(err).To(HaveOccurred()) - err = errors.Cause(err) - Expect(expectedErr).To(Equal(err)) - }) - It("no conf, fill conf error", func() { - r.ptestRegister(ptestNewImpl) - expectedErr := errors.New("fill conf err") - _, err := testNew(r, func(_ interface{}) error { return expectedErr }) - Expect(expectedErr).To(Equal(err)) - }) - It("no default", func() { - r.ptestRegister(func(c ptestConfig) *ptestImpl { return &ptestImpl{c.Value} }) - Expect(testNewOk()).To(Equal("")) - }) - It("default", func() { - r.ptestRegister(ptestNewConf, ptestDefaultConf) - Expect(testNewOk()).To(Equal(ptestDefaultValue)) - }) - It("fill conf default", func() { - r.ptestRegister(ptestNewConf, ptestDefaultConf) - Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) - }) - It("fill conf no default", func() { - r.ptestRegister(ptestNewConf) - Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) - }) - It("fill ptr conf no default", func() { - r.ptestRegister(ptestNewPtrConf) - Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) - }) - It("no default ptr conf not nil", func() { - r.ptestRegister(ptestNewPtrConf) - Expect("").To(Equal(testNewOk())) - }) - It("nil default, conf not nil", func() { - r.ptestRegister(ptestNewPtrConf, func() *ptestConfig { return nil }) - Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) - }) - It("fill nil default", func() { - r.ptestRegister(ptestNewPtrConf, func() *ptestConfig { return nil }) - Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) }) - It("more than one fill conf panics", func() { - r.ptestRegister(ptestNewPtrConf) - defer recoverExpectationFail() - testNew(r, ptestFillConf, ptestFillConf) + + Context("factory constructor", func() { + It("no conf", func() { + r.ptestRegister(ptestNewFactory) + Expect(testNewOk()).To(Equal(ptestInitValue)) + }) + It("nil error", func() { + r.ptestRegister(func() (ptestPlugin, error) { + return ptestNewImpl(), nil + }) + Expect(testNewOk()).To(Equal(ptestInitValue)) + }) + It("non-nil error", func() { + r.ptestRegister(ptestNewFactoryFactoryErrFailing) + _, err := testNew(r) + Expect(err).To(HaveOccurred()) + err = errors.Cause(err) + Expect(ptestCreateFailedErr).To(Equal(err)) + }) + It("no conf, fill conf error", func() { + r.ptestRegister(ptestNewFactory) + expectedErr := errors.New("fill conf err") + _, err := testNew(r, func(_ interface{}) error { return expectedErr }) + Expect(expectedErr).To(Equal(err)) + }) + It("no default", func() { + r.ptestRegister(ptestNewFactoryConf) + Expect(testNewOk()).To(Equal("")) + }) + It("default", func() { + r.ptestRegister(ptestNewFactoryConf, ptestDefaultConf) + Expect(testNewOk()).To(Equal(ptestDefaultValue)) + }) + It("fill conf default", func() { + r.ptestRegister(ptestNewFactoryConf, ptestDefaultConf) + Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) + }) + It("fill conf no default", func() { + r.ptestRegister(ptestNewFactoryConf) + Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) + }) + It("fill ptr conf no default", func() { + r.ptestRegister(ptestNewFactoryPtrConf) + Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) + }) + It("no default ptr conf not nil", func() { + r.ptestRegister(ptestNewFactoryPtrConf) + Expect("").To(Equal(testNewOk())) + }) + It("nil default, conf not nil", func() { + r.ptestRegister(ptestNewFactoryPtrConf, func() *ptestConfig { return nil }) + Expect(testNewOk()).To(Equal("")) + }) + It("fill nil default", func() { + r.ptestRegister(ptestNewFactoryPtrConf, func() *ptestConfig { return nil }) + Expect(testNewOk(ptestFillConf)).To(Equal(ptestFilledValue)) + }) + It("more than one fill conf panics", func() { + r.ptestRegister(ptestNewFactoryPtrConf) + defer recoverExpectationFail() + testNew(r, ptestFillConf, ptestFillConf) + }) }) } Context("use New", func() { From bc61bf6af559eef3998a9605423eabf37b68f227 Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Sun, 1 Oct 2017 14:08:24 +0300 Subject: [PATCH 13/15] Throw panic in config.Map --- components/phttp/client.go | 10 ++-------- core/config/config.go | 9 ++++++--- core/config/config_test.go | 12 ++++-------- 3 files changed, 12 insertions(+), 19 deletions(-) diff --git a/components/phttp/client.go b/components/phttp/client.go index 79c8c86f1..0f72d2466 100644 --- a/components/phttp/client.go +++ b/components/phttp/client.go @@ -71,10 +71,7 @@ func (f DialerFunc) DialContext(ctx context.Context, network, address string) (n func NewDialer(conf DialerConfig) *net.Dialer { d := &net.Dialer{} - err := config.Map(d, conf) - if err != nil { - zap.L().Panic("Dialer config map fail", zap.Error(err)) - } + config.Map(d, conf) return d } @@ -107,10 +104,7 @@ func NewTransport(conf TransportConfig, dial DialerFunc) *http.Transport { InsecureSkipVerify: true, // We should not spend time for this stuff. NextProtos: []string{"http/1.1"}, // Disable HTTP/2. Use HTTP/2 transport explicitly, if needed. } - err := config.Map(tr, conf) - if err != nil { - zap.L().Panic("Transport config map fail", zap.Error(err)) - } + config.Map(tr, conf) tr.DialContext = dial return tr } diff --git a/core/config/config.go b/core/config/config.go index 2cd507f89..3f2203fc5 100644 --- a/core/config/config.go +++ b/core/config/config.go @@ -54,7 +54,7 @@ func AddKindHook(hook KindHook) (_ struct{}) { // Example: you need to configure only some subset fields of struct Multi, // in such case you can from this subset of fields struct Single, decode config // into it, and map it on Multi. -func Map(dst, src interface{}) error { +func Map(dst, src interface{}) { conf := &mapstructure.DecoderConfig{ ErrorUnused: true, ZeroFields: true, @@ -62,11 +62,14 @@ func Map(dst, src interface{}) error { } d, err := mapstructure.NewDecoder(conf) if err != nil { - return err + panic(err) } s := structs.New(src) s.TagName = "map" - return d.Decode(s.Map()) + err = d.Decode(s.Map()) + if err != nil { + panic(err) + } } func newDecoderConfig(result interface{}) *mapstructure.DecoderConfig { diff --git a/core/config/config_test.go b/core/config/config_test.go index 3ec92160b..a602c6bc1 100644 --- a/core/config/config_test.go +++ b/core/config/config_test.go @@ -151,13 +151,11 @@ type SingleString struct { func TestMapFlat(t *testing.T) { a := &MultiStrings{} - err := Map(a, &SingleString{B: "b"}) - require.NoError(t, err) + Map(a, &SingleString{B: "b"}) assert.Equal(t, &MultiStrings{B: "b"}, a) a = &MultiStrings{A: "a", B: "not b"} - err = Map(a, &SingleString{B: "b"}) - require.NoError(t, err) + Map(a, &SingleString{B: "b"}) assert.Equal(t, &MultiStrings{A: "a", B: "b"}, a) } @@ -170,8 +168,7 @@ func TestMapRecursive(t *testing.T) { MultiStrings } n := &N{MultiStrings: MultiStrings{B: "b"}, A: "a"} - err := Map(n, &M{MultiStrings: MultiStrings{A: "a"}}) - require.NoError(t, err) + Map(n, &M{MultiStrings: MultiStrings{A: "a"}}) assert.Equal(t, &N{A: "a", MultiStrings: MultiStrings{A: "a"}}, n) } @@ -184,8 +181,7 @@ func TestMapTagged(t *testing.T) { SomeOtherFieldName MultiStrings `map:"MultiStrings"` } n := &N{MultiStrings: MultiStrings{B: "b"}, A: "a"} - err := Map(n, &M{SomeOtherFieldName: MultiStrings{A: "a"}}) - require.NoError(t, err) + Map(n, &M{SomeOtherFieldName: MultiStrings{A: "a"}}) assert.Equal(t, &N{A: "a", MultiStrings: MultiStrings{A: "a"}}, n) } From 2363e410cebbf1f738dd1506ec425f6ee633bf48 Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Sun, 1 Oct 2017 19:19:11 +0300 Subject: [PATCH 14/15] Resolve gun targets in phttp component once before shooting. --- .../example/import/import_suite_test.go | 20 +++ components/phttp/client.go | 27 ++-- components/phttp/connect.go | 3 +- components/phttp/http.go | 2 +- components/phttp/import/import.go | 58 ++++++- components/phttp/import/import_suite_test.go | 72 +++++++++ core/import/import_suite_test.go | 8 +- lib/netutil/dial.go | 115 ++++++++++++++ lib/netutil/mocks/conn.go | 142 ++++++++++++++++++ lib/netutil/mocks/dialer.go | 34 +++++ lib/netutil/mocks/dns_cache.go | 35 +++++ lib/netutil/netutil_suite_test.go | 107 +++++++++++++ lib/testutil/ginkgo.go | 9 ++ script/coverage.sh | 2 +- 14 files changed, 602 insertions(+), 32 deletions(-) create mode 100644 components/example/import/import_suite_test.go create mode 100644 components/phttp/import/import_suite_test.go create mode 100644 lib/netutil/dial.go create mode 100644 lib/netutil/mocks/conn.go create mode 100644 lib/netutil/mocks/dialer.go create mode 100644 lib/netutil/mocks/dns_cache.go create mode 100644 lib/netutil/netutil_suite_test.go diff --git a/components/example/import/import_suite_test.go b/components/example/import/import_suite_test.go new file mode 100644 index 000000000..06002b770 --- /dev/null +++ b/components/example/import/import_suite_test.go @@ -0,0 +1,20 @@ +package example + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/yandex/pandora/lib/testutil" +) + +func TestImport(t *testing.T) { + testutil.RunSuite(t, "Import Suite") +} + +var _ = Describe("import", func() { + It("not panics", func() { + Expect(Import).NotTo(Panic()) + }) +}) diff --git a/components/phttp/client.go b/components/phttp/client.go index 0f72d2466..7ee02d409 100644 --- a/components/phttp/client.go +++ b/components/phttp/client.go @@ -6,7 +6,6 @@ package phttp import ( - "context" "crypto/tls" "net" "net/http" @@ -17,6 +16,7 @@ import ( "github.com/pkg/errors" "github.com/yandex/pandora/core/config" + "github.com/yandex/pandora/lib/netutil" ) //go:generate mockery -name=Client -case=underscore -inpkg -testonly @@ -43,6 +43,8 @@ func NewDefaultClientConfig() ClientConfig { // DialerConfig can be mapped on net.Dialer. // Set net.Dialer for details. type DialerConfig struct { + DNSCache bool `config:"dns-cache" map:"-"` + Timeout time.Duration `config:"timeout"` DualStack bool `config:"dual-stack"` @@ -54,25 +56,20 @@ type DialerConfig struct { func NewDefaultDialerConfig() DialerConfig { return DialerConfig{ + DNSCache: true, + DualStack: true, Timeout: 3 * time.Second, KeepAlive: 120 * time.Second, } } -type Dialer interface { - DialContext(ctx context.Context, network, address string) (net.Conn, error) -} - -type DialerFunc func(ctx context.Context, network, address string) (net.Conn, error) - -func (f DialerFunc) DialContext(ctx context.Context, network, address string) (net.Conn, error) { - return f(ctx, network, address) -} - -func NewDialer(conf DialerConfig) *net.Dialer { +func NewDialer(conf DialerConfig) netutil.Dialer { d := &net.Dialer{} config.Map(d, conf) - return d + if !conf.DNSCache { + return d + } + return netutil.NewDNSCachingDialer(d, netutil.DefaultDNSCache) } // TransportConfig can be mapped on http.Transport. @@ -98,7 +95,7 @@ func NewDefaultTransportConfig() TransportConfig { } } -func NewTransport(conf TransportConfig, dial DialerFunc) *http.Transport { +func NewTransport(conf TransportConfig, dial netutil.DialerFunc) *http.Transport { tr := &http.Transport{} tr.TLSClientConfig = &tls.Config{ InsecureSkipVerify: true, // We should not spend time for this stuff. @@ -109,7 +106,7 @@ func NewTransport(conf TransportConfig, dial DialerFunc) *http.Transport { return tr } -func NewHTTP2Transport(conf TransportConfig, dial DialerFunc) *http.Transport { +func NewHTTP2Transport(conf TransportConfig, dial netutil.DialerFunc) *http.Transport { tr := NewTransport(conf, dial) err := http2.ConfigureTransport(tr) if err != nil { diff --git a/components/phttp/connect.go b/components/phttp/connect.go index 4ea48b289..a05a3d8ec 100644 --- a/components/phttp/connect.go +++ b/components/phttp/connect.go @@ -15,6 +15,7 @@ import ( "net/url" "github.com/pkg/errors" + "github.com/yandex/pandora/lib/netutil" ) type ConnectGunConfig struct { @@ -78,7 +79,7 @@ func newConnectClient(conf ConnectGunConfig) Client { return newClient(transport, conf.Client.Redirect) } -func newConnectDialFunc(target string, connectSSL bool, dialer Dialer) DialerFunc { +func newConnectDialFunc(target string, connectSSL bool, dialer netutil.Dialer) netutil.DialerFunc { return func(ctx context.Context, network, address string) (conn net.Conn, err error) { // TODO(skipor): make connect sample. // TODO(skipor): make httptrace callbacks called correctly. diff --git a/components/phttp/http.go b/components/phttp/http.go index 1b33c313a..f043a6d86 100644 --- a/components/phttp/http.go +++ b/components/phttp/http.go @@ -36,7 +36,7 @@ func NewHTTPGun(conf HTTPGunConfig) *HTTPGun { // NewHTTP2Gun return simple HTTP/2 gun that can shoot sequentially through one connection. func NewHTTP2Gun(conf HTTP2GunConfig) (*HTTPGun, error) { if !conf.Gun.SSL { - // Open issue on github if you need this feature. + // Open issue on github if you really need this feature. return nil, errors.New("HTTP/2.0 over TCP is not supported. Please leave SSL option true by default.") } transport := NewHTTP2Transport(conf.Client.Transport, NewDialer(conf.Client.Dialer).DialContext) diff --git a/components/phttp/import/import.go b/components/phttp/import/import.go index 5ea87f375..9733cf5bf 100644 --- a/components/phttp/import/import.go +++ b/components/phttp/import/import.go @@ -6,7 +6,10 @@ package phttp import ( + "net" + "github.com/spf13/afero" + "go.uber.org/zap" . "github.com/yandex/pandora/components/phttp" "github.com/yandex/pandora/components/phttp/ammo/simple/jsonline" @@ -14,6 +17,7 @@ import ( "github.com/yandex/pandora/components/phttp/ammo/simple/uri" "github.com/yandex/pandora/core" "github.com/yandex/pandora/core/register" + "github.com/yandex/pandora/lib/netutil" ) func Import(fs afero.Fs) { @@ -29,16 +33,56 @@ func Import(fs afero.Fs) { return raw.NewProvider(fs, conf) }) - register.Gun("http", func(conf HTTPGunConfig) core.Gun { - return WrapGun(NewHTTPGun(conf)) + register.Gun("http", func(conf HTTPGunConfig) func() core.Gun { + preResolveTargetAddr(&conf.Client, &conf.Gun.Target) + return func() core.Gun { return WrapGun(NewHTTPGun(conf)) } }, NewDefaultHTTPGunConfig) - register.Gun("http2", func(conf HTTP2GunConfig) (core.Gun, error) { - gun, err := NewHTTP2Gun(conf) - return WrapGun(gun), err + register.Gun("http2", func(conf HTTP2GunConfig) func() (core.Gun, error) { + preResolveTargetAddr(&conf.Client, &conf.Gun.Target) + return func() (core.Gun, error) { + gun, err := NewHTTP2Gun(conf) + return WrapGun(gun), err + } }, NewDefaultHTTP2GunConfig) - register.Gun("connect", func(conf ConnectGunConfig) core.Gun { - return WrapGun(NewConnectGun(conf)) + register.Gun("connect", func(conf ConnectGunConfig) func() core.Gun { + preResolveTargetAddr(&conf.Client, &conf.Target) + return func() core.Gun { + return WrapGun(NewConnectGun(conf)) + } }, NewDefaultConnectGunConfig) } + +// DNS resolve optimisation. +// When DNSCache turned off - do nothing extra, host will be resolved on every shoot. +// When using resolved target, don't use DNS caching logic - it is useless. +// If we can resolve accessible target addr - use it as target, not use caching. +// Otherwise just use DNS cache - we should not fail shooting, we should try to +// connect on every shoot. DNS cache will save resolved addr after first successful connect. +func preResolveTargetAddr(clientConf *ClientConfig, target *string) (err error) { + if !clientConf.Dialer.DNSCache { + return + } + if endpointIsResolved(*target) { + clientConf.Dialer.DNSCache = false + return + } + resolved, err := netutil.LookupReachable(*target) + if err != nil { + zap.L().Warn("DNS target pre resolve failed", + zap.String("target", *target), zap.Error(err)) + return + } + clientConf.Dialer.DNSCache = false + *target = resolved + return +} + +func endpointIsResolved(endpoint string) bool { + host, _, err := net.SplitHostPort(endpoint) + if err != nil { + return false + } + return net.ParseIP(host) != nil +} diff --git a/components/phttp/import/import_suite_test.go b/components/phttp/import/import_suite_test.go new file mode 100644 index 000000000..b8432e4c2 --- /dev/null +++ b/components/phttp/import/import_suite_test.go @@ -0,0 +1,72 @@ +package phttp + +import ( + "net" + "strconv" + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/spf13/afero" + + . "github.com/yandex/pandora/components/phttp" + "github.com/yandex/pandora/lib/testutil" +) + +func TestImport(t *testing.T) { + testutil.RunSuite(t, "phttp Import Suite") +} + +var _ = Describe("import", func() { + It("not panics", func() { + Expect(func() { + Import(afero.NewOsFs()) + }).NotTo(Panic()) + }) +}) + +var _ = Describe("preResolveTargetAddr", func() { + It("host target", func() { + conf := &ClientConfig{} + conf.Dialer.DNSCache = true + + listener, err := net.ListenTCP("tcp6", nil) + defer listener.Close() + Expect(err).NotTo(HaveOccurred()) + + port := strconv.Itoa(listener.Addr().(*net.TCPAddr).Port) + target := "localhost:" + port + expectedResolved := "[::1]:" + port + + err = preResolveTargetAddr(conf, &target) + Expect(err).NotTo(HaveOccurred()) + Expect(conf.Dialer.DNSCache).To(BeFalse()) + + Expect(target).To(Equal(expectedResolved)) + }) + + It("ip target", func() { + conf := &ClientConfig{} + conf.Dialer.DNSCache = true + + const addr = "127.0.0.1:80" + target := addr + err := preResolveTargetAddr(conf, &target) + Expect(err).NotTo(HaveOccurred()) + Expect(conf.Dialer.DNSCache).To(BeFalse()) + Expect(target).To(Equal(addr)) + }) + + It("failed", func() { + conf := &ClientConfig{} + conf.Dialer.DNSCache = true + + const addr = "localhost:54321" + target := addr + err := preResolveTargetAddr(conf, &target) + Expect(err).To(HaveOccurred()) + Expect(conf.Dialer.DNSCache).To(BeTrue()) + Expect(target).To(Equal(addr)) + }) + +}) diff --git a/core/import/import_suite_test.go b/core/import/import_suite_test.go index cb41a2952..77a71b008 100644 --- a/core/import/import_suite_test.go +++ b/core/import/import_suite_test.go @@ -6,7 +6,6 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/onsi/gomega/format" "github.com/spf13/afero" "github.com/yandex/pandora/core" @@ -16,13 +15,8 @@ import ( ) func TestImport(t *testing.T) { - format.UseStringerRepresentation = true - RegisterFailHandler(Fail) - - testutil.ReplaceGlobalLogger() Import(afero.NewOsFs()) - - RunSpecs(t, "Import Suite") + testutil.RunSuite(t, "Import Suite") } var _ = Describe("plugin decode", func() { diff --git a/lib/netutil/dial.go b/lib/netutil/dial.go new file mode 100644 index 000000000..44c94b0e1 --- /dev/null +++ b/lib/netutil/dial.go @@ -0,0 +1,115 @@ +// Copyright (c) 2017 Yandex LLC. All rights reserved. +// Use of this source code is governed by a MPL 2.0 +// license that can be found in the LICENSE file. +// Author: Vladimir Skipor + +package netutil + +import ( + "context" + "net" + "sync" + + "github.com/pkg/errors" +) + +//go:generate mockery -name=Dialer -case=underscore -outpkg=netmock + +type Dialer interface { + DialContext(ctx context.Context, net, addr string) (net.Conn, error) +} + +var _ Dialer = &net.Dialer{} + +type DialerFunc func(ctx context.Context, network, address string) (net.Conn, error) + +func (f DialerFunc) DialContext(ctx context.Context, network, address string) (net.Conn, error) { + return f(ctx, network, address) +} + +// NewDNSCachingDialer returns dialer with primitive DNS caching logic +// that remembers remote address on first try, and use it in future. +func NewDNSCachingDialer(dialer Dialer, cache DNSCache) DialerFunc { + return func(ctx context.Context, network, addr string) (conn net.Conn, err error) { + resolved, ok := cache.Get(addr) + if ok { + return dialer.DialContext(ctx, network, resolved) + } + conn, err = dialer.DialContext(ctx, network, addr) + if err != nil { + return + } + remoteAddr := conn.RemoteAddr().(*net.TCPAddr) + _, port, err := net.SplitHostPort(addr) + if err != nil { + conn.Close() + return nil, errors.Wrap(err, "invalid address, but successful dial - should not happen") + } + cache.Add(addr, net.JoinHostPort(remoteAddr.IP.String(), port)) + return + } +} + +var DefaultDNSCache = &SimpleDNSCache{} + +// LookupReachable tries to resolve addr via connecting to it. +// This method has much more overhead, but get guaranteed reachable resolved addr. +// Example: host is resolved to IPv4 and IPv6, but IPv4 is not working on machine. +// LookupAccessible will return IPv6 in that case. +func LookupReachable(addr string) (string, error) { + d := net.Dialer{DualStack: true} + conn, err := d.Dial("tcp", addr) + if err != nil { + return "", err + } + defer conn.Close() + _, port, err := net.SplitHostPort(addr) + if err != nil { + return "", err + } + remoteAddr := conn.RemoteAddr().(*net.TCPAddr) + return net.JoinHostPort(remoteAddr.IP.String(), port), nil +} + +// WarmDNSCache tries connect to addr, and adds conn remote ip + addr port to cache. +func WarmDNSCache(c DNSCache, addr string) error { + var d net.Dialer + conn, err := NewDNSCachingDialer(&d, c).DialContext(context.Background(), "tcp", addr) + if err != nil { + return err + } + conn.Close() + return nil +} + +//go:generate mockery -name=DNSCache -case=underscore -outpkg=netmock + +type DNSCache interface { + Get(addr string) (string, bool) + Add(addr, resolved string) +} + +type SimpleDNSCache struct { + rw sync.RWMutex + hostToAddr map[string]string +} + +func (c *SimpleDNSCache) Get(addr string) (resolved string, ok bool) { + c.rw.RLock() + if c.hostToAddr == nil { + c.rw.RUnlock() + return + } + resolved, ok = c.hostToAddr[addr] + c.rw.RUnlock() + return +} + +func (c *SimpleDNSCache) Add(addr, resolved string) { + c.rw.Lock() + if c.hostToAddr == nil { + c.hostToAddr = make(map[string]string) + } + c.hostToAddr[addr] = resolved + c.rw.Unlock() +} diff --git a/lib/netutil/mocks/conn.go b/lib/netutil/mocks/conn.go new file mode 100644 index 000000000..7f10c79c0 --- /dev/null +++ b/lib/netutil/mocks/conn.go @@ -0,0 +1,142 @@ +// Code generated by mockery v1.0.0 +package netmock + +import "github.com/stretchr/testify/mock" +import "net" + +import "time" + +// Conn is an autogenerated mock type for the Conn type +type Conn struct { + mock.Mock +} + +// Close provides a mock function with given fields: +func (_m *Conn) Close() error { + ret := _m.Called() + + var r0 error + if rf, ok := ret.Get(0).(func() error); ok { + r0 = rf() + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// LocalAddr provides a mock function with given fields: +func (_m *Conn) LocalAddr() net.Addr { + ret := _m.Called() + + var r0 net.Addr + if rf, ok := ret.Get(0).(func() net.Addr); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(net.Addr) + } + } + + return r0 +} + +// Read provides a mock function with given fields: b +func (_m *Conn) Read(b []byte) (int, error) { + ret := _m.Called(b) + + var r0 int + if rf, ok := ret.Get(0).(func([]byte) int); ok { + r0 = rf(b) + } else { + r0 = ret.Get(0).(int) + } + + var r1 error + if rf, ok := ret.Get(1).(func([]byte) error); ok { + r1 = rf(b) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// RemoteAddr provides a mock function with given fields: +func (_m *Conn) RemoteAddr() net.Addr { + ret := _m.Called() + + var r0 net.Addr + if rf, ok := ret.Get(0).(func() net.Addr); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(net.Addr) + } + } + + return r0 +} + +// SetDeadline provides a mock function with given fields: t +func (_m *Conn) SetDeadline(t time.Time) error { + ret := _m.Called(t) + + var r0 error + if rf, ok := ret.Get(0).(func(time.Time) error); ok { + r0 = rf(t) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// SetReadDeadline provides a mock function with given fields: t +func (_m *Conn) SetReadDeadline(t time.Time) error { + ret := _m.Called(t) + + var r0 error + if rf, ok := ret.Get(0).(func(time.Time) error); ok { + r0 = rf(t) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// SetWriteDeadline provides a mock function with given fields: t +func (_m *Conn) SetWriteDeadline(t time.Time) error { + ret := _m.Called(t) + + var r0 error + if rf, ok := ret.Get(0).(func(time.Time) error); ok { + r0 = rf(t) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// Write provides a mock function with given fields: b +func (_m *Conn) Write(b []byte) (int, error) { + ret := _m.Called(b) + + var r0 int + if rf, ok := ret.Get(0).(func([]byte) int); ok { + r0 = rf(b) + } else { + r0 = ret.Get(0).(int) + } + + var r1 error + if rf, ok := ret.Get(1).(func([]byte) error); ok { + r1 = rf(b) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} diff --git a/lib/netutil/mocks/dialer.go b/lib/netutil/mocks/dialer.go new file mode 100644 index 000000000..27e1ea755 --- /dev/null +++ b/lib/netutil/mocks/dialer.go @@ -0,0 +1,34 @@ +// Code generated by mockery v1.0.0 +package netmock + +import "context" +import "github.com/stretchr/testify/mock" +import "net" + +// Dialer is an autogenerated mock type for the Dialer type +type Dialer struct { + mock.Mock +} + +// DialContext provides a mock function with given fields: ctx, _a1, addr +func (_m *Dialer) DialContext(ctx context.Context, _a1 string, addr string) (net.Conn, error) { + ret := _m.Called(ctx, _a1, addr) + + var r0 net.Conn + if rf, ok := ret.Get(0).(func(context.Context, string, string) net.Conn); ok { + r0 = rf(ctx, _a1, addr) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(net.Conn) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, string, string) error); ok { + r1 = rf(ctx, _a1, addr) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} diff --git a/lib/netutil/mocks/dns_cache.go b/lib/netutil/mocks/dns_cache.go new file mode 100644 index 000000000..ca9979966 --- /dev/null +++ b/lib/netutil/mocks/dns_cache.go @@ -0,0 +1,35 @@ +// Code generated by mockery v1.0.0 +package netmock + +import "github.com/stretchr/testify/mock" + +// DNSCache is an autogenerated mock type for the DNSCache type +type DNSCache struct { + mock.Mock +} + +// Add provides a mock function with given fields: addr, resolved +func (_m *DNSCache) Add(addr string, resolved string) { + _m.Called(addr, resolved) +} + +// Get provides a mock function with given fields: addr +func (_m *DNSCache) Get(addr string) (string, bool) { + ret := _m.Called(addr) + + var r0 string + if rf, ok := ret.Get(0).(func(string) string); ok { + r0 = rf(addr) + } else { + r0 = ret.Get(0).(string) + } + + var r1 bool + if rf, ok := ret.Get(1).(func(string) bool); ok { + r1 = rf(addr) + } else { + r1 = ret.Get(1).(bool) + } + + return r0, r1 +} diff --git a/lib/netutil/netutil_suite_test.go b/lib/netutil/netutil_suite_test.go new file mode 100644 index 000000000..462bb073c --- /dev/null +++ b/lib/netutil/netutil_suite_test.go @@ -0,0 +1,107 @@ +package netutil + +import ( + "context" + "net" + "strconv" + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/pkg/errors" + "github.com/yandex/pandora/lib/netutil/mocks" + "github.com/yandex/pandora/lib/testutil" +) + +func TestNetutil(t *testing.T) { + testutil.RunSuite(t, "Netutil Suite") +} + +var _ = Describe("DNS", func() { + + It("lookup reachable", func() { + listener, err := net.ListenTCP("tcp6", nil) + defer listener.Close() + Expect(err).NotTo(HaveOccurred()) + + port := strconv.Itoa(listener.Addr().(*net.TCPAddr).Port) + addr := "localhost:" + port + expectedResolved := "[::1]:" + port + + resolved, err := LookupReachable(addr) + Expect(err).NotTo(HaveOccurred()) + Expect(resolved).To(Equal(expectedResolved)) + }) + + const ( + addr = "localhost:8888" + resolved = "[::1]:8888" + ) + + It("cache", func() { + cache := &SimpleDNSCache{} + got, ok := cache.Get(addr) + Expect(ok).To(BeFalse()) + Expect(got).To(BeEmpty()) + + cache.Add(addr, resolved) + got, ok = cache.Get(addr) + Expect(ok).To(BeTrue()) + Expect(got).To(Equal(resolved)) + }) + + It("Dialer cache miss", func() { + ctx := context.Background() + mockConn := &netmock.Conn{} + mockConn.On("RemoteAddr").Return(&net.TCPAddr{ + IP: net.IPv6loopback, + Port: 8888, + }) + cache := &netmock.DNSCache{} + cache.On("Get", addr).Return("", false) + cache.On("Add", addr, resolved) + dialer := &netmock.Dialer{} + dialer.On("DialContext", ctx, "tcp", addr).Return(mockConn, nil) + + testee := NewDNSCachingDialer(dialer, cache) + conn, err := testee.DialContext(ctx, "tcp", addr) + Expect(err).NotTo(HaveOccurred()) + Expect(conn).To(Equal(mockConn)) + + testutil.AssertExpectations(mockConn, cache, dialer) + }) + + It("Dialer cache hit", func() { + ctx := context.Background() + mockConn := &netmock.Conn{} + cache := &netmock.DNSCache{} + cache.On("Get", addr).Return(resolved, true) + dialer := &netmock.Dialer{} + dialer.On("DialContext", ctx, "tcp", resolved).Return(mockConn, nil) + + testee := NewDNSCachingDialer(dialer, cache) + conn, err := testee.DialContext(ctx, "tcp", addr) + Expect(err).NotTo(HaveOccurred()) + Expect(conn).To(Equal(mockConn)) + + testutil.AssertExpectations(mockConn, cache, dialer) + }) + + It("Dialer cache miss err", func() { + ctx := context.Background() + expectedErr := errors.New("dial failed") + cache := &netmock.DNSCache{} + cache.On("Get", addr).Return("", false) + dialer := &netmock.Dialer{} + dialer.On("DialContext", ctx, "tcp", addr).Return(nil, expectedErr) + + testee := NewDNSCachingDialer(dialer, cache) + conn, err := testee.DialContext(ctx, "tcp", addr) + Expect(err).To(Equal(expectedErr)) + Expect(conn).To(BeNil()) + + testutil.AssertExpectations(cache, dialer) + }) + +}) diff --git a/lib/testutil/ginkgo.go b/lib/testutil/ginkgo.go index b60339da6..888227f33 100644 --- a/lib/testutil/ginkgo.go +++ b/lib/testutil/ginkgo.go @@ -7,15 +7,24 @@ package testutil import ( "strings" + "testing" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/onsi/gomega/format" "github.com/spf13/viper" "github.com/stretchr/testify/mock" "go.uber.org/zap" "go.uber.org/zap/zapcore" ) +func RunSuite(t *testing.T, description string) { + format.UseStringerRepresentation = true + ReplaceGlobalLogger() + RegisterFailHandler(Fail) + RunSpecs(t, description) +} + func ReplaceGlobalLogger() *zap.Logger { log := NewLogger() zap.ReplaceGlobals(log) diff --git a/script/coverage.sh b/script/coverage.sh index 1a7fdf5c7..a40d4ae37 100755 --- a/script/coverage.sh +++ b/script/coverage.sh @@ -14,7 +14,7 @@ _cd_into_top_level() { } _generate_coverage_files() { - for dir in $(find . -maxdepth 10 -not -path './.git*' -not -path '*/vendor/*' -type d); do + for dir in $(find . -maxdepth 10 -not -path './.git*' -not -path '*/vendor/*' -not -path '*/mocks/*' -type d); do if ls $dir/*.go &>/dev/null ; then go test -covermode=count -coverprofile=$dir/profile.coverprofile $dir || fail=1 fi From 8e5dccc9b2adae1f6f98a54709bba5f24f454246 Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Sun, 1 Oct 2017 20:29:31 +0300 Subject: [PATCH 15/15] Fix failing LookupReachable travis tests --- components/phttp/import/import_suite_test.go | 4 ++-- lib/netutil/netutil_suite_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/components/phttp/import/import_suite_test.go b/components/phttp/import/import_suite_test.go index b8432e4c2..0c7f0f60d 100644 --- a/components/phttp/import/import_suite_test.go +++ b/components/phttp/import/import_suite_test.go @@ -30,13 +30,13 @@ var _ = Describe("preResolveTargetAddr", func() { conf := &ClientConfig{} conf.Dialer.DNSCache = true - listener, err := net.ListenTCP("tcp6", nil) + listener, err := net.ListenTCP("tcp4", nil) defer listener.Close() Expect(err).NotTo(HaveOccurred()) port := strconv.Itoa(listener.Addr().(*net.TCPAddr).Port) target := "localhost:" + port - expectedResolved := "[::1]:" + port + expectedResolved := "127.0.0.1:" + port err = preResolveTargetAddr(conf, &target) Expect(err).NotTo(HaveOccurred()) diff --git a/lib/netutil/netutil_suite_test.go b/lib/netutil/netutil_suite_test.go index 462bb073c..f36a31599 100644 --- a/lib/netutil/netutil_suite_test.go +++ b/lib/netutil/netutil_suite_test.go @@ -21,13 +21,13 @@ func TestNetutil(t *testing.T) { var _ = Describe("DNS", func() { It("lookup reachable", func() { - listener, err := net.ListenTCP("tcp6", nil) + listener, err := net.ListenTCP("tcp4", nil) defer listener.Close() Expect(err).NotTo(HaveOccurred()) port := strconv.Itoa(listener.Addr().(*net.TCPAddr).Port) addr := "localhost:" + port - expectedResolved := "[::1]:" + port + expectedResolved := "127.0.0.1:" + port resolved, err := LookupReachable(addr) Expect(err).NotTo(HaveOccurred())