From 428dce8aa7705606d3bca74c22b3aa25c1e32a30 Mon Sep 17 00:00:00 2001 From: Reasno Date: Wed, 22 Sep 2021 11:22:34 +0800 Subject: [PATCH 1/4] fix: Optimized debug message --- c.go | 14 ++++++++++---- di/graph.go | 6 ++++++ di/graph_test.go | 18 ++++++++++++++++++ go.mod | 2 +- go.sum | 4 ++-- 5 files changed, 37 insertions(+), 7 deletions(-) diff --git a/c.go b/c.go index 035c6f42..2dbf2c9e 100644 --- a/c.go +++ b/c.go @@ -8,10 +8,8 @@ package core import ( "context" - "errors" "fmt" "reflect" - "regexp" "github.com/DoNewsCode/core/codec/yaml" "github.com/DoNewsCode/core/config" @@ -309,6 +307,16 @@ func (c *C) provide(constructor interface{}) { } return filteredOuts }) + if pcProvider, ok := c.di.(interface { + ProvideWithPC(ctor interface{}, pc uintptr) error + }); ok { + pc := reflect.ValueOf(constructor).Pointer() + err := pcProvider.ProvideWithPC(fn.Interface(), pc) + if err != nil { + panic(err) + } + return + } err := c.di.Provide(fn.Interface()) if err != nil { panic(err) @@ -408,8 +416,6 @@ func (c *C) AddModuleFunc(constructor interface{}) { func (c *C) Invoke(function interface{}) { err := c.di.Invoke(function) if err != nil { - re := regexp.MustCompile(` missing dependencies for function "reflect"\.makeFuncStub \(.+?\):`) - err = errors.New(re.ReplaceAllString(err.Error(), "")) panic(err) } } diff --git a/di/graph.go b/di/graph.go index d2d6fa0b..aa2ada3f 100644 --- a/di/graph.go +++ b/di/graph.go @@ -40,6 +40,12 @@ func (g *Graph) Provide(constructor interface{}) error { return g.dig.Provide(constructor) } +// ProvideWithPC is like Provide, but additionally parametrized with a pointer +// for the constructor. This is useful when constructing the debug message. +func (g *Graph) ProvideWithPC(constructor interface{}, pc uintptr) error { + return g.dig.Provide(constructor, dig.LocationForPC(pc)) +} + // Invoke runs the given function after instantiating its dependencies. Any // arguments that the function has are treated as its dependencies. The // dependencies are instantiated in an unspecified order along with any diff --git a/di/graph_test.go b/di/graph_test.go index a91b56de..4d1786d4 100644 --- a/di/graph_test.go +++ b/di/graph_test.go @@ -1,6 +1,8 @@ package di import ( + "reflect" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -47,3 +49,19 @@ func TestBind(t *testing.T) { }) assert.NoError(t, err) } + +func ctor(f Fooer) Stub { + return Stub{} +} + +func TestProvideWithPC(t *testing.T) { + other := func(f Fooer) Stub { + return Stub{} + } + g := NewGraph() + g.ProvideWithPC(other, reflect.ValueOf(ctor).Pointer()) + err := g.Invoke(func(f Stub) {}) + if !strings.Contains(err.Error(), "missing dependencies for function \"github.com/DoNewsCode/core/di\".ctor") { + t.Errorf("ProvideWithPC should replace the passed in function with the function pc points to. got \"%s\"", err) + } +} diff --git a/go.mod b/go.mod index 110912b7..d5f30f74 100644 --- a/go.mod +++ b/go.mod @@ -36,7 +36,7 @@ require ( go.etcd.io/etcd/client/v3 v3.5.0 go.mongodb.org/mongo-driver v1.5.1 go.uber.org/atomic v1.7.0 - go.uber.org/dig v1.10.0 + go.uber.org/dig v1.13.0 go.uber.org/zap v1.17.0 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c google.golang.org/grpc v1.38.0 diff --git a/go.sum b/go.sum index b7619ea8..1e41b0ab 100644 --- a/go.sum +++ b/go.sum @@ -710,8 +710,8 @@ go.uber.org/atomic v1.5.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ= go.uber.org/atomic v1.6.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ= go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= -go.uber.org/dig v1.10.0 h1:yLmDDj9/zuDjv3gz8GQGviXMs9TfysIUMUilCpgzUJY= -go.uber.org/dig v1.10.0/go.mod h1:X34SnWGr8Fyla9zQNO2GSO2D+TIuqB14OS8JhYocIyw= +go.uber.org/dig v1.13.0 h1:bb9lVW3gtpQsNb07d0xL5vFwsjHidPJxaR/zSsbmfVQ= +go.uber.org/dig v1.13.0/go.mod h1:X34SnWGr8Fyla9zQNO2GSO2D+TIuqB14OS8JhYocIyw= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= go.uber.org/multierr v1.3.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4= go.uber.org/multierr v1.5.0/go.mod h1:FeouvMocqHpRaaGuG9EjoKcStLC43Zu/fmqdUMPcKYU= From b74f695296126e119f29376ddaad0b714eb7505b Mon Sep 17 00:00:00 2001 From: Reasno Date: Wed, 22 Sep 2021 16:29:15 +0800 Subject: [PATCH 2/4] refactor: remove di abstraction --- c.go | 31 ++++---- contract/di.go | 20 +---- default_config.go | 6 +- di/graph.go | 139 ----------------------------------- di/graph_test.go | 67 ----------------- di/optional_provider.go | 68 +++++++++++++++++ di/optional_provider_test.go | 95 ++++++++++++++++++++++++ di/populator.go | 75 +++++++++++++++++++ di/populator_test.go | 29 ++++++++ leader/dependency_test.go | 7 +- otes/dependency_test.go | 3 +- 11 files changed, 292 insertions(+), 248 deletions(-) delete mode 100644 di/graph.go delete mode 100644 di/graph_test.go create mode 100644 di/optional_provider.go create mode 100644 di/optional_provider_test.go create mode 100644 di/populator.go create mode 100644 di/populator_test.go diff --git a/c.go b/c.go index 2dbf2c9e..2be73e5a 100644 --- a/c.go +++ b/c.go @@ -21,6 +21,7 @@ import ( "github.com/go-kit/kit/log" "github.com/knadh/koanf/providers/confmap" "github.com/knadh/koanf/providers/file" + "go.uber.org/dig" ) // C stands for the core of the application. It contains service definitions and @@ -33,7 +34,7 @@ type C struct { logging.LevelLogger contract.Container contract.Dispatcher - di contract.DIContainer + di *dig.Container } // ConfParser models a parser for configuration. For example, yaml.Parser. @@ -55,7 +56,7 @@ type ConfigProvider func(configStack []config.ProviderSet, configWatcher contrac type EventDispatcherProvider func(conf contract.ConfigUnmarshaler) contract.Dispatcher // DiProvider provides the DiContainer to the core. -type DiProvider func(conf contract.ConfigUnmarshaler) contract.DIContainer +type DiProvider func(conf contract.ConfigUnmarshaler) *dig.Container // AppNameProvider provides the contract.AppName to the core. type AppNameProvider func(conf contract.ConfigUnmarshaler) contract.AppName @@ -247,8 +248,15 @@ func (c *C) Provide(deps di.Deps) { } func (c *C) provide(constructor interface{}) { - - var shouldMakeFunc bool + var ( + options []dig.ProvideOption + shouldMakeFunc bool + ) + + if op, ok := constructor.(di.OptionalProvider); ok { + constructor = op.Constructor + options = op.Options + } ftype := reflect.TypeOf(constructor) if ftype == nil { @@ -282,7 +290,7 @@ func (c *C) provide(constructor interface{}) { // no cleanup or module, we can use normal dig. if !shouldMakeFunc { - err := c.di.Provide(constructor) + err := c.di.Provide(constructor, options...) if err != nil { panic(err) } @@ -307,17 +315,8 @@ func (c *C) provide(constructor interface{}) { } return filteredOuts }) - if pcProvider, ok := c.di.(interface { - ProvideWithPC(ctor interface{}, pc uintptr) error - }); ok { - pc := reflect.ValueOf(constructor).Pointer() - err := pcProvider.ProvideWithPC(fn.Interface(), pc) - if err != nil { - panic(err) - } - return - } - err := c.di.Provide(fn.Interface()) + options = append(options, dig.LocationForPC(reflect.ValueOf(constructor).Pointer())) + err := c.di.Provide(fn.Interface(), options...) if err != nil { panic(err) } diff --git a/contract/di.go b/contract/di.go index 4de058c7..1c02309b 100644 --- a/contract/di.go +++ b/contract/di.go @@ -1,28 +1,10 @@ package contract -// DIProvider is an interface that models a container to which users can add -// dependencies. See di.Graph for the implementation requirement. -type DIProvider interface { - Provide(constructor interface{}) error -} - -// DIInvoker is an interface that models a container to which users can fetch -// dependencies. See di.Graph for the implementation requirement. -type DIInvoker interface { - Invoke(function interface{}) error -} - // DIPopulator is an interface that models a container to which users can fetch -// dependencies. It is an syntax sugar to DIInvoker. See di.Graph for the +// dependencies. It is an syntax sugar to dig.Container. See dig.Container for the // implementation requirement. type DIPopulator interface { // Populate is just another way of fetching dependencies from container. It // accepts a ptr to target, and populates the target from the container. Populate(target interface{}) error } - -// DIContainer is a container roughly modeled after dig.Container. -type DIContainer interface { - DIProvider - DIInvoker -} diff --git a/default_config.go b/default_config.go index 859d275e..8f2e9d46 100644 --- a/default_config.go +++ b/default_config.go @@ -7,13 +7,13 @@ import ( "github.com/DoNewsCode/core/config" "github.com/DoNewsCode/core/contract" - "github.com/DoNewsCode/core/di" "github.com/DoNewsCode/core/events" "github.com/DoNewsCode/core/logging" "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" "github.com/knadh/koanf/parsers/yaml" "github.com/knadh/koanf/providers/rawbytes" + "go.uber.org/dig" ) const defaultConfig = ` @@ -100,8 +100,8 @@ func ProvideLogger(conf contract.ConfigUnmarshaler, appName contract.AppName, en } // ProvideDi is the default DiProvider for package Core. -func ProvideDi(conf contract.ConfigUnmarshaler) contract.DIContainer { - return di.NewGraph() +func ProvideDi(conf contract.ConfigUnmarshaler) *dig.Container { + return dig.New() } // ProvideEventDispatcher is the default EventDispatcherProvider for package Core. diff --git a/di/graph.go b/di/graph.go deleted file mode 100644 index aa2ada3f..00000000 --- a/di/graph.go +++ /dev/null @@ -1,139 +0,0 @@ -// Package di is a thin wrapper around dig. See https://github.com/uber-go/dig -// -// This package is not intended for direct usage. Only use it for libraries -// written for package core. -package di - -import ( - "fmt" - "reflect" - "sync" - - "github.com/DoNewsCode/core/contract" - "go.uber.org/dig" -) - -// Graph is a wrapper around dig. -type Graph struct { - dig *dig.Container -} - -// NewGraph creates a graph -func NewGraph() *Graph { - return &Graph{dig: dig.New()} -} - -// Provide teaches the container how to build values of one or more types and -// expresses their dependencies. -// The first argument of Provide is a function that accepts zero or more -// parameters and returns one or more results. The function may optionally return -// an error to indicate that it failed to build the value. This function will be -// treated as the constructor for all the types it returns. This function will be -// called AT MOST ONCE when a type produced by it, or a type that consumes this -// function's output, is requested via Invoke. If the same types are requested -// multiple times, the previously produced value will be reused. In addition to -// accepting constructors that accept dependencies as separate arguments and -// produce results as separate return values, Provide also accepts constructors -// that specify dependencies as di.In structs and/or specify results as di.Out -// structs. -func (g *Graph) Provide(constructor interface{}) error { - return g.dig.Provide(constructor) -} - -// ProvideWithPC is like Provide, but additionally parametrized with a pointer -// for the constructor. This is useful when constructing the debug message. -func (g *Graph) ProvideWithPC(constructor interface{}, pc uintptr) error { - return g.dig.Provide(constructor, dig.LocationForPC(pc)) -} - -// Invoke runs the given function after instantiating its dependencies. Any -// arguments that the function has are treated as its dependencies. The -// dependencies are instantiated in an unspecified order along with any -// dependencies that they might have. The function may return an error to -// indicate failure. The error will be returned to the caller as-is. -func (g *Graph) Invoke(function interface{}) error { - return g.dig.Invoke(function) -} - -// String representation of the entire Container -func (g *Graph) String() string { - return g.dig.String() -} - -type defaultPopulator struct { - mutex sync.Mutex - invoker contract.DIInvoker -} - -// Populate sets targets with values from the dependency injection container -// during application initialization. All targets must be pointers to the -// values that must be populated. Pointers to structs that embed In are -// supported, which can be used to populate multiple values in a struct. -// -// This is most helpful in unit tests: it lets tests leverage Fx's automatic -// constructor wiring to build a few structs, but then extract those structs -// for further testing. -// -// Mostly copied from uber/fx. License: https://github.com/uber-go/fx/blob/master/LICENSE -func (d *defaultPopulator) Populate(target interface{}) error { - invokeErr := func(err error) error { - return d.invoker.Invoke(func() error { - return err - }) - } - targetTypes := make([]reflect.Type, 1) - // Validate all targets are non-nil pointers. - if target == nil { - return invokeErr(fmt.Errorf("failed to Populate: target is nil")) - } - rt := reflect.TypeOf(target) - if rt.Kind() != reflect.Ptr { - return invokeErr(fmt.Errorf("failed to Populate: target is not a pointer type, got %T", rt)) - } - - targetTypes[0] = rt.Elem() - - // Build a function that looks like: - // - // func(t1 T1, t2 T2, ...) { - // *targets[0] = t1 - // *targets[1] = t2 - // [...] - // } - // - fnType := reflect.FuncOf(targetTypes, nil, false /* variadic */) - fn := reflect.MakeFunc(fnType, func(args []reflect.Value) []reflect.Value { - for _, arg := range args { - reflect.ValueOf(target).Elem().Set(arg) - } - return nil - }) - - d.mutex.Lock() - defer d.mutex.Unlock() - return d.invoker.Invoke(fn.Interface()) -} - -// IntoPopulator converts a contract.DIInvoker to contract.DIPopulator. -func IntoPopulator(container contract.DIInvoker) contract.DIPopulator { - if populator, ok := container.(contract.DIPopulator); ok { - return populator - } - return &defaultPopulator{ - invoker: container, - } -} - -// Bind binds a type to another. Useful when binding implementation to -// interfaces. The arguments should be a ptr to the binding types, rather than -// the types themselves. For example: -// Bind(new(MyConcreteStruct), new(MyAbstractInterface)) -func Bind(from interface{}, to interface{}) interface{} { - fromTypes := []reflect.Type{reflect.TypeOf(from).Elem()} - toTypes := []reflect.Type{reflect.TypeOf(to).Elem()} - fnType := reflect.FuncOf(fromTypes, toTypes, false /* variadic */) - fn := reflect.MakeFunc(fnType, func(args []reflect.Value) []reflect.Value { - return args - }) - return fn.Interface() -} diff --git a/di/graph_test.go b/di/graph_test.go deleted file mode 100644 index 4d1786d4..00000000 --- a/di/graph_test.go +++ /dev/null @@ -1,67 +0,0 @@ -package di - -import ( - "reflect" - "strings" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestIntoPopulator(t *testing.T) { - var target int - g := NewGraph() - g.Provide(func() int { return 1 }) - - p := IntoPopulator(g) - err := p.Populate(&target) - assert.NoError(t, err) - assert.Equal(t, 1, target) - - err = p.Populate(nil) - assert.Error(t, err) - - var s string - err = p.Populate(&s) - assert.Error(t, err) - - err = p.Populate(s) - assert.Error(t, err) -} - -type Stub struct{} - -func (s Stub) Foo() {} - -type Fooer interface { - Foo() -} - -func TestBind(t *testing.T) { - ctor := func() Stub { - return Stub{} - } - g := NewGraph() - g.Provide(ctor) - g.Provide(Bind(new(Stub), new(Fooer))) - err := g.Invoke(func(f Fooer) { - assert.NotNil(t, f) - }) - assert.NoError(t, err) -} - -func ctor(f Fooer) Stub { - return Stub{} -} - -func TestProvideWithPC(t *testing.T) { - other := func(f Fooer) Stub { - return Stub{} - } - g := NewGraph() - g.ProvideWithPC(other, reflect.ValueOf(ctor).Pointer()) - err := g.Invoke(func(f Stub) {}) - if !strings.Contains(err.Error(), "missing dependencies for function \"github.com/DoNewsCode/core/di\".ctor") { - t.Errorf("ProvideWithPC should replace the passed in function with the function pc points to. got \"%s\"", err) - } -} diff --git a/di/optional_provider.go b/di/optional_provider.go new file mode 100644 index 00000000..c566c342 --- /dev/null +++ b/di/optional_provider.go @@ -0,0 +1,68 @@ +package di + +import ( + "reflect" + + "go.uber.org/dig" +) + +// OptionalProvider is a constructor with dig options. +type OptionalProvider struct { + Constructor interface{} + Options []dig.ProvideOption +} + +// LocationForPC sets the constructor pointer to a specified location. Use this +// Options to reduce vague debug message when constructor are made by +// reflect.makeFunc. +// LocationForPC(reflect.makeFunc(...), reflect.ValueOf(realConstructor).Pointer()) +func LocationForPC(constructor interface{}, pc uintptr) interface{} { + if op, ok := constructor.(OptionalProvider); ok { + op.Options = append(op.Options, dig.LocationForPC(pc)) + return op + } + return OptionalProvider{ + Constructor: constructor, + Options: []dig.ProvideOption{dig.LocationForPC(pc)}, + } +} + +// As constructs the instance and bind it to another interface. As is mean to be used as an argument to graph.Provide. +// As(MyConstructor, new(MyAbstractInterface)) +func As(constructor interface{}, as interface{}) interface{} { + if op, ok := constructor.(OptionalProvider); ok { + op.Options = append(op.Options, dig.As(as)) + return op + } + return OptionalProvider{ + Constructor: constructor, + Options: []dig.ProvideOption{dig.As(as)}, + } +} + +// Name constructs a named instance. Name is mean to be used as an argument to graph.Provide. +// Name(MyConstructor, "foo") +func Name(constructor interface{}, name string) interface{} { + if op, ok := constructor.(OptionalProvider); ok { + op.Options = append(op.Options, dig.Name(name)) + return op + } + return OptionalProvider{ + Constructor: constructor, + Options: []dig.ProvideOption{dig.Name(name)}, + } +} + +// Bind binds a type to another. Useful when binding implementation to +// interfaces. The arguments should be a ptr to the binding types, rather than +// the types themselves. For example: +// Bind(new(MyConcreteStruct), new(MyAbstractInterface)) +func Bind(from interface{}, to interface{}) interface{} { + fromTypes := []reflect.Type{reflect.TypeOf(from).Elem()} + toTypes := []reflect.Type{reflect.TypeOf(to).Elem()} + fnType := reflect.FuncOf(fromTypes, toTypes, false /* variadic */) + fn := reflect.MakeFunc(fnType, func(args []reflect.Value) []reflect.Value { + return args + }) + return fn.Interface() +} diff --git a/di/optional_provider_test.go b/di/optional_provider_test.go new file mode 100644 index 00000000..e2910f53 --- /dev/null +++ b/di/optional_provider_test.go @@ -0,0 +1,95 @@ +package di_test + +import ( + "reflect" + "testing" + + "github.com/DoNewsCode/core" + "github.com/DoNewsCode/core/di" + "github.com/stretchr/testify/assert" + "go.uber.org/dig" +) + +type Stub struct{} + +func (s Stub) Foo() {} + +type Fooer interface { + Foo() +} + +func ctor() Stub { + return Stub{} +} + +func badConstructor(f Fooer) Stub { + return Stub{} +} + +func TestBind(t *testing.T) { + ctor := func() Stub { + return Stub{} + } + g := core.New() + g.Provide(di.Deps{ctor, di.Bind(new(Stub), new(Fooer))}) + g.Invoke(func(f Fooer) { + assert.NotNil(t, f) + }) +} + +func TestName(t *testing.T) { + g := dig.New() + err := g.Provide(di.Name(ctor, "foo")) + assert.NoError(t, err) + err = g.Invoke(func(injected struct { + dig.In + Stub Stub `name:"foo"` + }) { + }) + assert.NoError(t, err) +} + +func TestAs(t *testing.T) { + g := core.New() + g.Provide(di.Deps{di.As(ctor, new(Fooer))}) + g.Invoke(func(injected struct { + dig.In + Stub Fooer + }) { + }) +} + +func TestLocationForPC(t *testing.T) { + + g := core.New() + inlineCtor := func(f Fooer) Stub { + return Stub{} + } + g.Provide(di.Deps{di.LocationForPC(inlineCtor, reflect.ValueOf(badConstructor).Pointer())}) + + defer func() { + if r := recover(); r != nil { + assert.Contains(t, r.(error).Error(), "badConstructor") + return + } + t.Fatal("test should panic") + }() + + g.Invoke(func(injected struct { + dig.In + Stub Stub + }) { + }) +} + +func TestChainedOptions(t *testing.T) { + g := core.New() + g.Provide(di.Deps{di.As(di.Name(di.LocationForPC(ctor, reflect.ValueOf(ctor).Pointer()), "foo"), new(Fooer))}) + + g.Invoke(func(injected struct { + di.In + Stub Fooer `name:"foo"` + }) { + }) + +} diff --git a/di/populator.go b/di/populator.go new file mode 100644 index 00000000..10646089 --- /dev/null +++ b/di/populator.go @@ -0,0 +1,75 @@ +// Package di is a thin wrapper around dig. See https://github.com/uber-go/dig +// +// This package is not intended for direct usage. Only use it for libraries +// written for package core. +package di + +import ( + "fmt" + "reflect" + "sync" + + "github.com/DoNewsCode/core/contract" + "go.uber.org/dig" +) + +type defaultPopulator struct { + mutex sync.Mutex + invoker *dig.Container +} + +// Populate sets targets with values from the dependency injection container +// during application initialization. All targets must be pointers to the +// values that must be populated. Pointers to structs that embed In are +// supported, which can be used to populate multiple values in a struct. +// +// This is most helpful in unit tests: it lets tests leverage Fx's automatic +// constructor wiring to build a few structs, but then extract those structs +// for further testing. +// +// Mostly copied from uber/fx. License: https://github.com/uber-go/fx/blob/master/LICENSE +func (d *defaultPopulator) Populate(target interface{}) error { + invokeErr := func(err error) error { + return d.invoker.Invoke(func() error { + return err + }) + } + targetTypes := make([]reflect.Type, 1) + // Validate all targets are non-nil pointers. + if target == nil { + return invokeErr(fmt.Errorf("failed to Populate: target is nil")) + } + rt := reflect.TypeOf(target) + if rt.Kind() != reflect.Ptr { + return invokeErr(fmt.Errorf("failed to Populate: target is not a pointer type, got %T", rt)) + } + + targetTypes[0] = rt.Elem() + + // Build a function that looks like: + // + // func(t1 T1, t2 T2, ...) { + // *targets[0] = t1 + // *targets[1] = t2 + // [...] + // } + // + fnType := reflect.FuncOf(targetTypes, nil, false /* variadic */) + fn := reflect.MakeFunc(fnType, func(args []reflect.Value) []reflect.Value { + for _, arg := range args { + reflect.ValueOf(target).Elem().Set(arg) + } + return nil + }) + + d.mutex.Lock() + defer d.mutex.Unlock() + return d.invoker.Invoke(fn.Interface()) +} + +// IntoPopulator converts a contract.DIInvoker to contract.DIPopulator. +func IntoPopulator(container *dig.Container) contract.DIPopulator { + return &defaultPopulator{ + invoker: container, + } +} diff --git a/di/populator_test.go b/di/populator_test.go new file mode 100644 index 00000000..b3bc9e04 --- /dev/null +++ b/di/populator_test.go @@ -0,0 +1,29 @@ +package di + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.uber.org/dig" +) + +func TestIntoPopulator(t *testing.T) { + var target int + g := dig.New() + g.Provide(func() int { return 1 }) + + p := IntoPopulator(g) + err := p.Populate(&target) + assert.NoError(t, err) + assert.Equal(t, 1, target) + + err = p.Populate(nil) + assert.Error(t, err) + + var s string + err = p.Populate(&s) + assert.Error(t, err) + + err = p.Populate(s) + assert.Error(t, err) +} diff --git a/leader/dependency_test.go b/leader/dependency_test.go index 6cc7928f..522ee851 100644 --- a/leader/dependency_test.go +++ b/leader/dependency_test.go @@ -15,6 +15,7 @@ import ( "github.com/DoNewsCode/core/otetcd" "github.com/stretchr/testify/assert" "go.etcd.io/etcd/client/v3" + "go.uber.org/dig" ) type mockMaker struct { @@ -76,7 +77,7 @@ type mockPopulator struct { } func (m mockPopulator) Populate(target interface{}) error { - c := di.NewGraph() + c := dig.New() c.Provide(func() contract.AppName { return config.AppName("foo") }) @@ -120,7 +121,7 @@ func Test_provideConfig(t *testing.T) { } func TestPreferDriverInDI(t *testing.T) { - g := di.NewGraph() + g := dig.New() g.Provide(func() Driver { return mockDriver{} }) @@ -132,7 +133,7 @@ func TestPreferDriverInDI(t *testing.T) { } func TestPreferDriverInDI_error(t *testing.T) { - g := di.NewGraph() + g := dig.New() g.Provide(func() (Driver, error) { return mockDriver{}, errors.New("err") }) diff --git a/otes/dependency_test.go b/otes/dependency_test.go index bb123335..303d6b76 100644 --- a/otes/dependency_test.go +++ b/otes/dependency_test.go @@ -11,12 +11,13 @@ import ( "github.com/olivere/elastic/v7" "github.com/opentracing/opentracing-go" "github.com/stretchr/testify/assert" + "go.uber.org/dig" ) type Populator struct{} func (p Populator) Populate(target interface{}) error { - g := di.NewGraph() + g := dig.New() g.Provide(func() log.Logger { return log.NewNopLogger() }) From babb917ad9c6330c909b022a51cb44f4fa684dab Mon Sep 17 00:00:00 2001 From: Reasno Date: Wed, 22 Sep 2021 16:33:32 +0800 Subject: [PATCH 3/4] test: fix failed tests --- di/optional_provider.go | 4 +++- di/optional_provider_test.go | 8 +++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/di/optional_provider.go b/di/optional_provider.go index c566c342..e21b1e71 100644 --- a/di/optional_provider.go +++ b/di/optional_provider.go @@ -6,7 +6,9 @@ import ( "go.uber.org/dig" ) -// OptionalProvider is a constructor with dig options. +// OptionalProvider is a struct with constructor and dig options. When +// OptionalProvider is used as the element in di.Deps, the options are applied to +// the inner dig.Container automatically. type OptionalProvider struct { Constructor interface{} Options []dig.ProvideOption diff --git a/di/optional_provider_test.go b/di/optional_provider_test.go index e2910f53..fd214559 100644 --- a/di/optional_provider_test.go +++ b/di/optional_provider_test.go @@ -38,15 +38,13 @@ func TestBind(t *testing.T) { } func TestName(t *testing.T) { - g := dig.New() - err := g.Provide(di.Name(ctor, "foo")) - assert.NoError(t, err) - err = g.Invoke(func(injected struct { + g := core.New() + g.Provide(di.Deps{di.Name(ctor, "foo")}) + g.Invoke(func(injected struct { dig.In Stub Stub `name:"foo"` }) { }) - assert.NoError(t, err) } func TestAs(t *testing.T) { From 89c438402c59161f7f6c085ec7d771d055a99b5c Mon Sep 17 00:00:00 2001 From: Trock Date: Wed, 22 Sep 2021 17:50:36 +0800 Subject: [PATCH 4/4] fix: comment --- c.go | 4 ++-- contract/di.go | 2 +- di/optional_provider.go | 8 +++++--- di/populator.go | 2 +- ots3/uploader.go | 4 ++-- 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/c.go b/c.go index 2be73e5a..1276ed4f 100644 --- a/c.go +++ b/c.go @@ -25,7 +25,7 @@ import ( ) // C stands for the core of the application. It contains service definitions and -// dependencies. C is mean to be used in the boostrap phase of the application. +// dependencies. C means to be used in the boostrap phase of the application. // Do not pass C into services and use it as a service locator. type C struct { AppName contract.AppName @@ -55,7 +55,7 @@ type ConfigProvider func(configStack []config.ProviderSet, configWatcher contrac // EventDispatcherProvider provides contract.Dispatcher to the core. type EventDispatcherProvider func(conf contract.ConfigUnmarshaler) contract.Dispatcher -// DiProvider provides the DiContainer to the core. +// DiProvider provides the *dig.Container to the core. type DiProvider func(conf contract.ConfigUnmarshaler) *dig.Container // AppNameProvider provides the contract.AppName to the core. diff --git a/contract/di.go b/contract/di.go index 1c02309b..46bca51d 100644 --- a/contract/di.go +++ b/contract/di.go @@ -1,7 +1,7 @@ package contract // DIPopulator is an interface that models a container to which users can fetch -// dependencies. It is an syntax sugar to dig.Container. See dig.Container for the +// dependencies. It is a syntax sugar to dig.Container. See dig.Container for the // implementation requirement. type DIPopulator interface { // Populate is just another way of fetching dependencies from container. It diff --git a/di/optional_provider.go b/di/optional_provider.go index e21b1e71..6d9d5e2c 100644 --- a/di/optional_provider.go +++ b/di/optional_provider.go @@ -16,7 +16,7 @@ type OptionalProvider struct { // LocationForPC sets the constructor pointer to a specified location. Use this // Options to reduce vague debug message when constructor are made by -// reflect.makeFunc. +// reflect.makeFunc. For example: // LocationForPC(reflect.makeFunc(...), reflect.ValueOf(realConstructor).Pointer()) func LocationForPC(constructor interface{}, pc uintptr) interface{} { if op, ok := constructor.(OptionalProvider); ok { @@ -29,7 +29,8 @@ func LocationForPC(constructor interface{}, pc uintptr) interface{} { } } -// As constructs the instance and bind it to another interface. As is mean to be used as an argument to graph.Provide. +// As constructs the instance and bind it to another interface. As means to be used as an argument to graph.Provide. +// For example: // As(MyConstructor, new(MyAbstractInterface)) func As(constructor interface{}, as interface{}) interface{} { if op, ok := constructor.(OptionalProvider); ok { @@ -42,7 +43,8 @@ func As(constructor interface{}, as interface{}) interface{} { } } -// Name constructs a named instance. Name is mean to be used as an argument to graph.Provide. +// Name constructs a named instance. Name means to be used as an argument to graph.Provide. +// For example: // Name(MyConstructor, "foo") func Name(constructor interface{}, name string) interface{} { if op, ok := constructor.(OptionalProvider); ok { diff --git a/di/populator.go b/di/populator.go index 10646089..626a9b3b 100644 --- a/di/populator.go +++ b/di/populator.go @@ -67,7 +67,7 @@ func (d *defaultPopulator) Populate(target interface{}) error { return d.invoker.Invoke(fn.Interface()) } -// IntoPopulator converts a contract.DIInvoker to contract.DIPopulator. +// IntoPopulator converts a *dig.Container to contract.DIPopulator. func IntoPopulator(container *dig.Container) contract.DIPopulator { return &defaultPopulator{ invoker: container, diff --git a/ots3/uploader.go b/ots3/uploader.go index b6504ba0..de82e564 100644 --- a/ots3/uploader.go +++ b/ots3/uploader.go @@ -34,7 +34,7 @@ type Manager struct { autoExtension bool } -// ManagerConfig contains a various of configurations for Manager. It is mean to be modified by ManagerOptionFunc. +// ManagerConfig contains a various configurations for Manager. It means to be modified by ManagerOptionFunc. type ManagerConfig struct { tracer opentracing.Tracer doer contract.HttpDoer @@ -75,7 +75,7 @@ func WithHTTPClient(client contract.HttpDoer) ManagerOptionFunc { } } -// WithLocationFunc is an option that decides the how url is mapped to S3 bucket and path. +// WithLocationFunc is an option that decides how url is mapped to S3 bucket and path. // Useful when not serving file directly from S3, but from a CDN. func WithLocationFunc(f func(location string) (url string)) ManagerOptionFunc { return func(c *ManagerConfig) {