From 039a6c6fc07487ffeef94493f464051a62f04517 Mon Sep 17 00:00:00 2001 From: Jing Chen Date: Wed, 22 Feb 2017 13:49:26 -0500 Subject: [PATCH 1/7] Return error if multiple implementations are present for one interface --- inject.go | 62 ++++++++++++++++++++++++++----------------- inject_test.go | 72 +++++++++++++++++++++++++++++++++++--------------- 2 files changed, 89 insertions(+), 45 deletions(-) diff --git a/inject.go b/inject.go index 3ff713c..136775b 100644 --- a/inject.go +++ b/inject.go @@ -49,7 +49,7 @@ type TypeMapper interface { Set(reflect.Type, reflect.Value) TypeMapper // Returns the Value that is mapped to the current type. Returns a zeroed Value if // the Type has not been mapped. - Get(reflect.Type) reflect.Value + Get(reflect.Type) (reflect.Value, error) } type injector struct { @@ -91,7 +91,10 @@ func (inj *injector) Invoke(f interface{}) ([]reflect.Value, error) { var in = make([]reflect.Value, t.NumIn()) //Panic if t is not kind of Func for i := 0; i < t.NumIn(); i++ { argType := t.In(i) - val := inj.Get(argType) + val, err := inj.Get(argType) + if err != nil { + return nil, err + } if !val.IsValid() { return nil, fmt.Errorf("Value not found for type %v", argType) } @@ -123,7 +126,10 @@ func (inj *injector) Apply(val interface{}) error { structField := t.Field(i) if f.CanSet() && (structField.Tag == "inject" || structField.Tag.Get("inject") != "") { ft := f.Type() - v := inj.Get(ft) + v, err := inj.Get(ft) + if err != nil { + return err + } if !v.IsValid() { return fmt.Errorf("Value not found for type %v", ft) } @@ -138,50 +144,58 @@ func (inj *injector) Apply(val interface{}) error { // Maps the concrete value of val to its dynamic type using reflect.TypeOf, // It returns the TypeMapper registered in. -func (i *injector) Map(val interface{}) TypeMapper { - i.values[reflect.TypeOf(val)] = reflect.ValueOf(val) - return i +func (inj *injector) Map(val interface{}) TypeMapper { + inj.values[reflect.TypeOf(val)] = reflect.ValueOf(val) + return inj } -func (i *injector) MapTo(val interface{}, ifacePtr interface{}) TypeMapper { - i.values[InterfaceOf(ifacePtr)] = reflect.ValueOf(val) - return i +func (inj *injector) MapTo(val interface{}, ifacePtr interface{}) TypeMapper { + inj.values[InterfaceOf(ifacePtr)] = reflect.ValueOf(val) + return inj } // Maps the given reflect.Type to the given reflect.Value and returns // the Typemapper the mapping has been registered in. -func (i *injector) Set(typ reflect.Type, val reflect.Value) TypeMapper { - i.values[typ] = val - return i +func (inj *injector) Set(typ reflect.Type, val reflect.Value) TypeMapper { + inj.values[typ] = val + return inj } -func (i *injector) Get(t reflect.Type) reflect.Value { - val := i.values[t] +func (inj *injector) Get(t reflect.Type) (reflect.Value, error) { + var err error + val := inj.values[t] if val.IsValid() { - return val + return val, nil } // no concrete types found, try to find implementors // if t is an interface + var impls []reflect.Value if t.Kind() == reflect.Interface { - for k, v := range i.values { + for k, v := range inj.values { if k.Implements(t) { - val = v - break + impls = append(impls, v) } } } + if len(impls) > 1 { + return val, fmt.Errorf("Expect single matching implementation but found %v", len(impls)) + } else if len(impls) == 1 { + val = impls[0] + } // Still no type found, try to look it up on the parent - if !val.IsValid() && i.parent != nil { - val = i.parent.Get(t) + if !val.IsValid() && inj.parent != nil { + val, err = inj.parent.Get(t) + if err != nil { + return val, err + } } - return val - + return val, nil } -func (i *injector) SetParent(parent Injector) { - i.parent = parent +func (inj *injector) SetParent(parent Injector) { + inj.parent = parent } diff --git a/inject_test.go b/inject_test.go index eb94471..29063b4 100644 --- a/inject_test.go +++ b/inject_test.go @@ -1,8 +1,7 @@ -package inject_test +package inject import ( "fmt" - "github.com/codegangsta/inject" "reflect" "testing" ) @@ -12,7 +11,7 @@ type SpecialString interface { type TestStruct struct { Dep1 string `inject:"t" json:"-"` - Dep2 SpecialString `inject` + Dep2 SpecialString `inject:"t"` Dep3 string } @@ -24,6 +23,14 @@ func (g *Greeter) String() string { return "Hello, My name is" + g.Name } +type Greeter2 struct { + Name string +} + +func (g *Greeter2) String() string { + return "Hello, My name is" + g.Name +} + /* Test Helpers */ func expect(t *testing.T, a interface{}, b interface{}) { if a != b { @@ -38,7 +45,7 @@ func refute(t *testing.T, a interface{}, b interface{}) { } func Test_InjectorInvoke(t *testing.T) { - injector := inject.New() + injector := New() expect(t, injector == nil, false) dep := "some dependency" @@ -65,7 +72,7 @@ func Test_InjectorInvoke(t *testing.T) { } func Test_InjectorInvokeReturnValues(t *testing.T) { - injector := inject.New() + injector := New() expect(t, injector == nil, false) dep := "some dependency" @@ -84,7 +91,7 @@ func Test_InjectorInvokeReturnValues(t *testing.T) { } func Test_InjectorApply(t *testing.T) { - injector := inject.New() + injector := New() injector.Map("a dep").MapTo("another dep", (*SpecialString)(nil)) @@ -98,10 +105,10 @@ func Test_InjectorApply(t *testing.T) { } func Test_InterfaceOf(t *testing.T) { - iType := inject.InterfaceOf((*SpecialString)(nil)) + iType := InterfaceOf((*SpecialString)(nil)) expect(t, iType.Kind(), reflect.Interface) - iType = inject.InterfaceOf((**SpecialString)(nil)) + iType = InterfaceOf((**SpecialString)(nil)) expect(t, iType.Kind(), reflect.Interface) // Expecting nil @@ -109,11 +116,11 @@ func Test_InterfaceOf(t *testing.T) { rec := recover() refute(t, rec, nil) }() - iType = inject.InterfaceOf((*testing.T)(nil)) + iType = InterfaceOf((*testing.T)(nil)) } func Test_InjectorSet(t *testing.T) { - injector := inject.New() + injector := New() typ := reflect.TypeOf("string") typSend := reflect.ChanOf(reflect.SendDir, typ) typRecv := reflect.ChanOf(reflect.RecvDir, typ) @@ -126,34 +133,57 @@ func Test_InjectorSet(t *testing.T) { injector.Set(typSend, chanSend) injector.Set(typRecv, chanRecv) - expect(t, injector.Get(typSend).IsValid(), true) - expect(t, injector.Get(typRecv).IsValid(), true) - expect(t, injector.Get(chanSend.Type()).IsValid(), false) + val, err := injector.Get(typSend) + expect(t, val.IsValid(), true) + expect(t, err, nil) + val, err = injector.Get(typRecv) + expect(t, val.IsValid(), true) + expect(t, err, nil) + val, err = injector.Get(chanSend.Type()) + expect(t, val.IsValid(), false) + expect(t, err, nil) } func Test_InjectorGet(t *testing.T) { - injector := inject.New() + injector := New() injector.Map("some dependency") - expect(t, injector.Get(reflect.TypeOf("string")).IsValid(), true) - expect(t, injector.Get(reflect.TypeOf(11)).IsValid(), false) + val, err := injector.Get(reflect.TypeOf("string")) + expect(t, val.IsValid(), true) + val, err = injector.Get(reflect.TypeOf(11)) + expect(t, val.IsValid(), false) + expect(t, err, nil) } func Test_InjectorSetParent(t *testing.T) { - injector := inject.New() + injector := New() injector.MapTo("another dep", (*SpecialString)(nil)) - injector2 := inject.New() + injector2 := New() injector2.SetParent(injector) - expect(t, injector2.Get(inject.InterfaceOf((*SpecialString)(nil))).IsValid(), true) + val, err := injector2.Get(InterfaceOf((*SpecialString)(nil))) + expect(t, val.IsValid(), true) + expect(t, err, nil) } func TestInjectImplementors(t *testing.T) { - injector := inject.New() + injector := New() g := &Greeter{"Jeremy"} injector.Map(g) - expect(t, injector.Get(inject.InterfaceOf((*fmt.Stringer)(nil))).IsValid(), true) + val, err := injector.Get(InterfaceOf((*fmt.Stringer)(nil))) + expect(t, val.IsValid(), true) + expect(t, err, nil) +} + +func TestInjectImplementors_AmbiguousImplementation(t *testing.T) { + injector := New() + g1, g2 := &Greeter{"Jeremy"}, &Greeter2{"Tom"} + injector.Map(g1).Map(g2) + + val, err := injector.Get(InterfaceOf((*fmt.Stringer)(nil))) + expect(t, val.IsValid(), false) + refute(t, err, nil) } From 52c3f2b60ec6305afd3a88cada251a49a45034cd Mon Sep 17 00:00:00 2001 From: Jing Chen Date: Wed, 22 Feb 2017 14:38:27 -0500 Subject: [PATCH 2/7] Revert the renaming of i to inj, and add a sentence to describe the change of Get method --- inject.go | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/inject.go b/inject.go index 136775b..1f1ed40 100644 --- a/inject.go +++ b/inject.go @@ -48,7 +48,8 @@ type TypeMapper interface { // with reflect like unidirectional channels. Set(reflect.Type, reflect.Value) TypeMapper // Returns the Value that is mapped to the current type. Returns a zeroed Value if - // the Type has not been mapped. + // the Type has not been mapped. An error will be returned in addition if there're + // more than one implementation for the type. Get(reflect.Type) (reflect.Value, error) } @@ -144,26 +145,26 @@ func (inj *injector) Apply(val interface{}) error { // Maps the concrete value of val to its dynamic type using reflect.TypeOf, // It returns the TypeMapper registered in. -func (inj *injector) Map(val interface{}) TypeMapper { - inj.values[reflect.TypeOf(val)] = reflect.ValueOf(val) - return inj +func (i *injector) Map(val interface{}) TypeMapper { + i.values[reflect.TypeOf(val)] = reflect.ValueOf(val) + return i } -func (inj *injector) MapTo(val interface{}, ifacePtr interface{}) TypeMapper { - inj.values[InterfaceOf(ifacePtr)] = reflect.ValueOf(val) - return inj +func (i *injector) MapTo(val interface{}, ifacePtr interface{}) TypeMapper { + i.values[InterfaceOf(ifacePtr)] = reflect.ValueOf(val) + return i } // Maps the given reflect.Type to the given reflect.Value and returns // the Typemapper the mapping has been registered in. -func (inj *injector) Set(typ reflect.Type, val reflect.Value) TypeMapper { - inj.values[typ] = val - return inj +func (i *injector) Set(typ reflect.Type, val reflect.Value) TypeMapper { + i.values[typ] = val + return i } -func (inj *injector) Get(t reflect.Type) (reflect.Value, error) { +func (i *injector) Get(t reflect.Type) (reflect.Value, error) { var err error - val := inj.values[t] + val := i.values[t] if val.IsValid() { return val, nil @@ -173,7 +174,7 @@ func (inj *injector) Get(t reflect.Type) (reflect.Value, error) { // if t is an interface var impls []reflect.Value if t.Kind() == reflect.Interface { - for k, v := range inj.values { + for k, v := range i.values { if k.Implements(t) { impls = append(impls, v) } @@ -186,8 +187,8 @@ func (inj *injector) Get(t reflect.Type) (reflect.Value, error) { } // Still no type found, try to look it up on the parent - if !val.IsValid() && inj.parent != nil { - val, err = inj.parent.Get(t) + if !val.IsValid() && i.parent != nil { + val, err = i.parent.Get(t) if err != nil { return val, err } From 14cee346431ee278ba561d3c43fbd95e44c44c10 Mon Sep 17 00:00:00 2001 From: Jing Chen Date: Wed, 22 Feb 2017 17:24:58 -0500 Subject: [PATCH 3/7] Revert the Get method signature change, use a configurable option to decide whether to panic on ambiguous implmementations --- inject.go | 54 +++++++++++++++++++++++++++----------------------- inject_test.go | 46 +++++++++++++++++++++--------------------- 2 files changed, 52 insertions(+), 48 deletions(-) diff --git a/inject.go b/inject.go index 1f1ed40..9b9dea4 100644 --- a/inject.go +++ b/inject.go @@ -16,6 +16,8 @@ type Injector interface { // dependency in its Type map it will check its parent before returning an // error. SetParent(Injector) + // SetOptions sets options to configure the injector. + SetOptions(InjectorOptions) } // Applicator represents an interface for mapping dependencies to a struct. @@ -48,14 +50,19 @@ type TypeMapper interface { // with reflect like unidirectional channels. Set(reflect.Type, reflect.Value) TypeMapper // Returns the Value that is mapped to the current type. Returns a zeroed Value if - // the Type has not been mapped. An error will be returned in addition if there're - // more than one implementation for the type. - Get(reflect.Type) (reflect.Value, error) + // the Type has not been mapped. + Get(reflect.Type) reflect.Value +} + +// InjectorOptions contains options to configure the injector +type InjectorOptions struct { + PanicOnAmbiguity bool } type injector struct { - values map[reflect.Type]reflect.Value - parent Injector + options InjectorOptions + values map[reflect.Type]reflect.Value + parent Injector } // InterfaceOf dereferences a pointer to an Interface type. @@ -92,10 +99,7 @@ func (inj *injector) Invoke(f interface{}) ([]reflect.Value, error) { var in = make([]reflect.Value, t.NumIn()) //Panic if t is not kind of Func for i := 0; i < t.NumIn(); i++ { argType := t.In(i) - val, err := inj.Get(argType) - if err != nil { - return nil, err - } + val := inj.Get(argType) if !val.IsValid() { return nil, fmt.Errorf("Value not found for type %v", argType) } @@ -127,10 +131,7 @@ func (inj *injector) Apply(val interface{}) error { structField := t.Field(i) if f.CanSet() && (structField.Tag == "inject" || structField.Tag.Get("inject") != "") { ft := f.Type() - v, err := inj.Get(ft) - if err != nil { - return err - } + v := inj.Get(ft) if !v.IsValid() { return fmt.Errorf("Value not found for type %v", ft) } @@ -162,12 +163,11 @@ func (i *injector) Set(typ reflect.Type, val reflect.Value) TypeMapper { return i } -func (i *injector) Get(t reflect.Type) (reflect.Value, error) { - var err error +func (i *injector) Get(t reflect.Type) reflect.Value { val := i.values[t] if val.IsValid() { - return val, nil + return val } // no concrete types found, try to find implementors @@ -181,22 +181,26 @@ func (i *injector) Get(t reflect.Type) (reflect.Value, error) { } } if len(impls) > 1 { - return val, fmt.Errorf("Expect single matching implementation but found %v", len(impls)) - } else if len(impls) == 1 { + if i.options.PanicOnAmbiguity { + panic(fmt.Sprintf("Expect single matching implementation but found %v", len(impls))) + } + } + if len(impls) > 0 { val = impls[0] } // Still no type found, try to look it up on the parent if !val.IsValid() && i.parent != nil { - val, err = i.parent.Get(t) - if err != nil { - return val, err - } + val = i.parent.Get(t) } - return val, nil + return val +} + +func (i *injector) SetParent(parent Injector) { + i.parent = parent } -func (inj *injector) SetParent(parent Injector) { - inj.parent = parent +func (inj *injector) SetOptions(options InjectorOptions) { + inj.options = options } diff --git a/inject_test.go b/inject_test.go index 29063b4..66d1953 100644 --- a/inject_test.go +++ b/inject_test.go @@ -133,15 +133,9 @@ func Test_InjectorSet(t *testing.T) { injector.Set(typSend, chanSend) injector.Set(typRecv, chanRecv) - val, err := injector.Get(typSend) - expect(t, val.IsValid(), true) - expect(t, err, nil) - val, err = injector.Get(typRecv) - expect(t, val.IsValid(), true) - expect(t, err, nil) - val, err = injector.Get(chanSend.Type()) - expect(t, val.IsValid(), false) - expect(t, err, nil) + expect(t, injector.Get(typSend).IsValid(), true) + expect(t, injector.Get(typRecv).IsValid(), true) + expect(t, injector.Get(chanSend.Type()).IsValid(), false) } func Test_InjectorGet(t *testing.T) { @@ -149,11 +143,8 @@ func Test_InjectorGet(t *testing.T) { injector.Map("some dependency") - val, err := injector.Get(reflect.TypeOf("string")) - expect(t, val.IsValid(), true) - val, err = injector.Get(reflect.TypeOf(11)) - expect(t, val.IsValid(), false) - expect(t, err, nil) + expect(t, injector.Get(reflect.TypeOf("string")).IsValid(), true) + expect(t, injector.Get(reflect.TypeOf(11)).IsValid(), false) } func Test_InjectorSetParent(t *testing.T) { @@ -163,9 +154,7 @@ func Test_InjectorSetParent(t *testing.T) { injector2 := New() injector2.SetParent(injector) - val, err := injector2.Get(InterfaceOf((*SpecialString)(nil))) - expect(t, val.IsValid(), true) - expect(t, err, nil) + expect(t, injector2.Get(InterfaceOf((*SpecialString)(nil))).IsValid(), true) } func TestInjectImplementors(t *testing.T) { @@ -173,9 +162,7 @@ func TestInjectImplementors(t *testing.T) { g := &Greeter{"Jeremy"} injector.Map(g) - val, err := injector.Get(InterfaceOf((*fmt.Stringer)(nil))) - expect(t, val.IsValid(), true) - expect(t, err, nil) + expect(t, injector.Get(InterfaceOf((*fmt.Stringer)(nil))).IsValid(), true) } func TestInjectImplementors_AmbiguousImplementation(t *testing.T) { @@ -183,7 +170,20 @@ func TestInjectImplementors_AmbiguousImplementation(t *testing.T) { g1, g2 := &Greeter{"Jeremy"}, &Greeter2{"Tom"} injector.Map(g1).Map(g2) - val, err := injector.Get(InterfaceOf((*fmt.Stringer)(nil))) - expect(t, val.IsValid(), false) - refute(t, err, nil) + expect(t, injector.Get(InterfaceOf((*fmt.Stringer)(nil))).IsValid(), true) +} + +func TestInjectImplementors_AmbiguousImplementationPanic(t *testing.T) { + defer func() { + r := recover() + expect(t, r, "Expect single matching implementation but found 2") + }() + + injector := New() + injector.SetOptions(InjectorOptions{ + PanicOnAmbiguity: true, + }) + g1, g2 := &Greeter{"Jeremy"}, &Greeter2{"Tom"} + injector.Map(g1).Map(g2) + expect(t, injector.Get(InterfaceOf((*fmt.Stringer)(nil))).IsValid(), true) } From 3f6847663785d50b7a3ee4738f2b6f07df5c2803 Mon Sep 17 00:00:00 2001 From: Jing Chen Date: Wed, 22 Feb 2017 17:27:49 -0500 Subject: [PATCH 4/7] Simplify code --- inject.go | 6 ++---- inject_test.go | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/inject.go b/inject.go index 9b9dea4..e57ac50 100644 --- a/inject.go +++ b/inject.go @@ -180,10 +180,8 @@ func (i *injector) Get(t reflect.Type) reflect.Value { } } } - if len(impls) > 1 { - if i.options.PanicOnAmbiguity { - panic(fmt.Sprintf("Expect single matching implementation but found %v", len(impls))) - } + if len(impls) > 1 && i.options.PanicOnAmbiguity { + panic(fmt.Sprintf("Expect single matching implementation but found %v", len(impls))) } if len(impls) > 0 { val = impls[0] diff --git a/inject_test.go b/inject_test.go index 66d1953..e39fdcc 100644 --- a/inject_test.go +++ b/inject_test.go @@ -11,7 +11,7 @@ type SpecialString interface { type TestStruct struct { Dep1 string `inject:"t" json:"-"` - Dep2 SpecialString `inject:"t"` + Dep2 SpecialString `inject` Dep3 string } @@ -185,5 +185,5 @@ func TestInjectImplementors_AmbiguousImplementationPanic(t *testing.T) { }) g1, g2 := &Greeter{"Jeremy"}, &Greeter2{"Tom"} injector.Map(g1).Map(g2) - expect(t, injector.Get(InterfaceOf((*fmt.Stringer)(nil))).IsValid(), true) + injector.Get(InterfaceOf((*fmt.Stringer)(nil))) } From ef3e0a817c43353df17626d1bdbe93eb8552f009 Mon Sep 17 00:00:00 2001 From: Jing Chen Date: Wed, 22 Feb 2017 17:52:13 -0500 Subject: [PATCH 5/7] Add more docs for PanicOnAmbiguity --- inject.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/inject.go b/inject.go index e57ac50..dfe7241 100644 --- a/inject.go +++ b/inject.go @@ -56,6 +56,8 @@ type TypeMapper interface { // InjectorOptions contains options to configure the injector type InjectorOptions struct { + // If PanicOnAmbiguity is set to true, Get method will panic if it finds multiple + // implementations that satisfy the given type. PanicOnAmbiguity bool } From 8eeec5b0e400faaaa52182d3400143724848f8c8 Mon Sep 17 00:00:00 2001 From: Jing Chen Date: Wed, 22 Feb 2017 18:31:25 -0500 Subject: [PATCH 6/7] Update readme (#2) Update README files --- README.md | 2 ++ translations/README_zh_cn.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/README.md b/README.md index 679abe0..88664af 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,8 @@ type Injector interface { // dependency in its Type map it will check its parent before returning an // error. SetParent(Injector) + // SetOptions sets options to configure the injector. + SetOptions(InjectorOptions) } ``` diff --git a/translations/README_zh_cn.md b/translations/README_zh_cn.md index 0ac3d3f..c9122ef 100644 --- a/translations/README_zh_cn.md +++ b/translations/README_zh_cn.md @@ -35,6 +35,8 @@ type Injector interface { // SetParent用来设置父injector. 如果在当前injector的Type map中找不到依赖， // 将会继续从它的父injector中找，直到返回error. SetParent(Injector) + // SetOptions提供一个接口用于设置injector. + SetOptions(InjectorOptions) } ``` From 78dabaf052f5d7462d16670c510dc22f4614d3a8 Mon Sep 17 00:00:00 2001 From: Jing Chen Date: Thu, 2 Mar 2017 22:35:43 -0500 Subject: [PATCH 7/7] Print out more info when panic (#3) * Print out more info when panic * Print out more info when panic on ambiguous implementations --- inject.go | 2 +- inject_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/inject.go b/inject.go index dfe7241..c3b60c6 100644 --- a/inject.go +++ b/inject.go @@ -183,7 +183,7 @@ func (i *injector) Get(t reflect.Type) reflect.Value { } } if len(impls) > 1 && i.options.PanicOnAmbiguity { - panic(fmt.Sprintf("Expect single matching implementation but found %v", len(impls))) + panic(fmt.Sprintf("Expected single matching implementation for type <%v> but found %v: %v", t, len(impls), impls)) } if len(impls) > 0 { val = impls[0] diff --git a/inject_test.go b/inject_test.go index e39fdcc..571d5e6 100644 --- a/inject_test.go +++ b/inject_test.go @@ -176,7 +176,7 @@ func TestInjectImplementors_AmbiguousImplementation(t *testing.T) { func TestInjectImplementors_AmbiguousImplementationPanic(t *testing.T) { defer func() { r := recover() - expect(t, r, "Expect single matching implementation but found 2") + expect(t, r, "Expected single matching implementation for type but found 2: [<*inject.Greeter Value> <*inject.Greeter2 Value>]") }() injector := New()