From 455b4d6f7960a5c2354f4d28d3f5f26bb4bcf46a Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Wed, 9 Feb 2022 16:51:31 -0800 Subject: [PATCH 1/4] fx.Replace added --- replace.go | 104 +++++++++++++++++++++++++++++++++++++ replace_test.go | 135 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 239 insertions(+) create mode 100644 replace.go create mode 100644 replace_test.go diff --git a/replace.go b/replace.go new file mode 100644 index 000000000..0325588e8 --- /dev/null +++ b/replace.go @@ -0,0 +1,104 @@ +// Copyright (c) 2022 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package fx + +import ( + "fmt" + "reflect" + "strings" + + "go.uber.org/fx/internal/fxreflect" +) + +// Replace provides instantiated values for dependency injection as if +// they had been provided using a constructor that simply returns them. +// The most specific type of each value (as determined by reflection) is used. +// Replace panics if a value (or annotation target) is an untyped nil or an error. +func Replace(values ...interface{}) Option { + decorators := make([]interface{}, len(values)) // one function per value + types := make([]reflect.Type, len(values)) + for i, value := range values { + switch value := value.(type) { + case annotated: + var typ reflect.Type + value.Target, typ = newReplaceDecorator(value.Target) + decorators[i] = value + types[i] = typ + case Annotated: + var typ reflect.Type + value.Target, typ = newReplaceDecorator(value.Target) + decorators[i] = value + types[i] = typ + default: + decorators[i], types[i] = newReplaceDecorator(value) + } + } + + return replaceOption{ + Targets: decorators, + Types: types, + Stack: fxreflect.CallerStack(1, 0), + } +} + +type replaceOption struct { + Targets []interface{} + Types []reflect.Type // type of value produced by constructor[i] + Stack fxreflect.Stack +} + +func (o replaceOption) apply(m *module) { + for _, target := range o.Targets { + m.decorators = append(m.decorators, decorator{ + Target: target, + Stack: o.Stack, + }) + } +} + +func (o replaceOption) String() string { + items := make([]string, 0, len(o.Targets)) + for _, typ := range o.Types { + items = append(items, typ.String()) + } + return fmt.Sprintf("fx.Replace(%s)", strings.Join(items, ", ")) +} + +// Returns a function that takes no parameters, and returns the given value. +func newReplaceDecorator(value interface{}) (interface{}, reflect.Type) { + switch value.(type) { + case nil: + panic("untyped nil passed to fx.Replace") + case error: + panic("error value passed to fx.Replace") + } + + typ := reflect.TypeOf(value) + returnTypes := []reflect.Type{typ} + returnValues := []reflect.Value{reflect.ValueOf(value)} + + ft := reflect.FuncOf([]reflect.Type{}, returnTypes, false) + fv := reflect.MakeFunc(ft, func([]reflect.Value) []reflect.Value { + return returnValues + }) + + return fv.Interface(), typ +} diff --git a/replace_test.go b/replace_test.go new file mode 100644 index 000000000..eeb37ee1a --- /dev/null +++ b/replace_test.go @@ -0,0 +1,135 @@ +// Copyright (c) 2022 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package fx_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.uber.org/fx" + "go.uber.org/fx/fxtest" +) + +func TestReplaceSuccess(t *testing.T) { + t.Parallel() + + t.Run("replace a value", func(t *testing.T) { + t.Parallel() + type A struct { + Value string + } + a := &A{Value: "a'"} + app := fxtest.New(t, + fx.Provide(func() *A { + return &A{ + Value: "a", + } + }), + fx.Replace(a), + fx.Invoke(func(a *A) { + assert.Equal(t, "a'", a.Value) + }), + ) + defer app.RequireStart().RequireStop() + }) + + t.Run("replace in a module", func(t *testing.T) { + t.Parallel() + + type A struct { + Value string + } + + a := &A{Value: "A"} + + app := fxtest.New(t, + fx.Module("child", + fx.Replace(a), + fx.Invoke(func(a *A) { + assert.Equal(t, "A", a.Value) + }), + ), + fx.Provide(func() *A { + return &A{ + Value: "a", + } + }), + ) + defer app.RequireStart().RequireStop() + }) +} + +func TestReplaceFailure(t *testing.T) { + t.Parallel() + + t.Run("replace same value twice", func(t *testing.T) { + t.Parallel() + + type A struct { + Value string + } + a := &A{Value: "A"} + app := NewForTest(t, + fx.Provide(func() *A { + return &A{Value: "a"} + }), + fx.Module("child", + fx.Replace(a), + fx.Replace(a), + fx.Invoke(func(a *A) { + assert.Fail(t, "this should never run") + }), + ), + ) + err := app.Err() + assert.Error(t, err) + assert.Contains(t, err.Error(), "*fx_test.A already decorated") + }) + + t.Run("replace with annotate", func(t *testing.T) { + t.Parallel() + + type A struct { + Value string + } + + app := fxtest.New(t, + fx.Supply( + fx.Annotate(A{"A"}, fx.ResultTags(`group:"t"`)), + fx.Annotate(A{"B"}, fx.ResultTags(`group:"t"`)), + fx.Annotate(A{"C"}, fx.ResultTags(`group:"t"`)), + ), + fx.Decorate( + fx.Annotate(func() A { return A{"a"} }, fx.ResultTags(`group:"t"`)), + ), + /* + fx.Replace( + fx.Annotate(A{"C"}, fx.ResultTags(`group:"t"`)), + fx.Annotate(A{"B"}, fx.ResultTags(`group:"t"`)), + ), + */ + fx.Invoke(fx.Annotate(func(a ...A) { + assert.ElementsMatch(t, []A{{"a"}, {"b"}, {"c"}}, a) + }, fx.ParamTags(`group:"t"`))), + ) + defer app.RequireStart().RequireStop() + }) +} From b40e1830d60112c48ac6216e2b7abee48d58e7ae Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Thu, 10 Feb 2022 14:19:44 -0800 Subject: [PATCH 2/4] more tests --- replace_test.go | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/replace_test.go b/replace_test.go index eeb37ee1a..1ff3bd2ad 100644 --- a/replace_test.go +++ b/replace_test.go @@ -113,21 +113,28 @@ func TestReplaceFailure(t *testing.T) { app := fxtest.New(t, fx.Supply( - fx.Annotate(A{"A"}, fx.ResultTags(`group:"t"`)), - fx.Annotate(A{"B"}, fx.ResultTags(`group:"t"`)), - fx.Annotate(A{"C"}, fx.ResultTags(`group:"t"`)), + fx.Annotate(A{"A"}, fx.ResultTags(`name:"t"`)), ), - fx.Decorate( - fx.Annotate(func() A { return A{"a"} }, fx.ResultTags(`group:"t"`)), + fx.Replace( + fx.Annotate(A{"B"}, fx.ResultTags(`name:"t"`)), ), - /* - fx.Replace( - fx.Annotate(A{"C"}, fx.ResultTags(`group:"t"`)), - fx.Annotate(A{"B"}, fx.ResultTags(`group:"t"`)), - ), - */ - fx.Invoke(fx.Annotate(func(a ...A) { - assert.ElementsMatch(t, []A{{"a"}, {"b"}, {"c"}}, a) + fx.Invoke(fx.Annotate(func(a A) { + assert.Equal(t, a.Value, "B") + }, fx.ParamTags(`name:"t"`))), + ) + defer app.RequireStart().RequireStop() + }) + + t.Run("replace a value group with annotate", func(t *testing.T) { + t.Parallel() + + app := fxtest.New(t, + fx.Supply( + fx.Annotate([]string{"A", "B", "C"}, fx.ResultTags(`group:"t"`)), + ), + fx.Replace(fx.Annotate([]string{"a", "b", "c"}, fx.ResultTags(`group:"t"`))), + fx.Invoke(fx.Annotate(func(ss ...string) { + assert.ElementsMatch(t, []string{"a", "b", "c"}, ss) }, fx.ParamTags(`group:"t"`))), ) defer app.RequireStart().RequireStop() From 90c4f3841716de6f6571073c0eb0ce02a62f12a1 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Thu, 10 Feb 2022 16:07:51 -0800 Subject: [PATCH 3/4] place the tests correctly --- app_test.go | 5 +++ replace.go | 15 ++++---- replace_test.go | 93 ++++++++++++++++++++++++++++++++++++------------- 3 files changed, 80 insertions(+), 33 deletions(-) diff --git a/app_test.go b/app_test.go index ad8d09582..48110d76d 100644 --- a/app_test.go +++ b/app_test.go @@ -1698,6 +1698,11 @@ func TestOptionString(t *testing.T) { give: Decorate(bytes.NewBufferString), want: "fx.Decorate(bytes.NewBufferString())", }, + { + desc: "Replace", + give: Replace(bytes.NewReader(nil)), + want: "fx.Replace(*bytes.Reader)", + }, } for _, tt := range tests { diff --git a/replace.go b/replace.go index 0325588e8..6d3b4d1a6 100644 --- a/replace.go +++ b/replace.go @@ -28,9 +28,13 @@ import ( "go.uber.org/fx/internal/fxreflect" ) -// Replace provides instantiated values for dependency injection as if -// they had been provided using a constructor that simply returns them. -// The most specific type of each value (as determined by reflection) is used. +// Replace provides instantiated values for graph modification. Similar to +// what fx.Supply is to fx.Provide, values provided by fx.Replace behaves +// similar to values produced by decorators specified with fx.Decorate. +// +// Refer to the documentation on fx.Decorate to see how graph modifications +// work with fx.Module. +// // Replace panics if a value (or annotation target) is an untyped nil or an error. func Replace(values ...interface{}) Option { decorators := make([]interface{}, len(values)) // one function per value @@ -42,11 +46,6 @@ func Replace(values ...interface{}) Option { value.Target, typ = newReplaceDecorator(value.Target) decorators[i] = value types[i] = typ - case Annotated: - var typ reflect.Type - value.Target, typ = newReplaceDecorator(value.Target) - decorators[i] = value - types[i] = typ default: decorators[i], types[i] = newReplaceDecorator(value) } diff --git a/replace_test.go b/replace_test.go index 1ff3bd2ad..e74c27236 100644 --- a/replace_test.go +++ b/replace_test.go @@ -21,9 +21,11 @@ package fx_test import ( + "errors" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/fx" "go.uber.org/fx/fxtest" ) @@ -75,6 +77,42 @@ func TestReplaceSuccess(t *testing.T) { ) defer app.RequireStart().RequireStop() }) + + t.Run("replace with annotate", func(t *testing.T) { + t.Parallel() + + type A struct { + Value string + } + + app := fxtest.New(t, + fx.Supply( + fx.Annotate(A{"A"}, fx.ResultTags(`name:"t"`)), + ), + fx.Replace( + fx.Annotate(A{"B"}, fx.ResultTags(`name:"t"`)), + ), + fx.Invoke(fx.Annotate(func(a A) { + assert.Equal(t, a.Value, "B") + }, fx.ParamTags(`name:"t"`))), + ) + defer app.RequireStart().RequireStop() + }) + + t.Run("replace a value group with annotate", func(t *testing.T) { + t.Parallel() + + app := fxtest.New(t, + fx.Supply( + fx.Annotate([]string{"A", "B", "C"}, fx.ResultTags(`group:"t"`)), + ), + fx.Replace(fx.Annotate([]string{"a", "b", "c"}, fx.ResultTags(`group:"t"`))), + fx.Invoke(fx.Annotate(func(ss ...string) { + assert.ElementsMatch(t, []string{"a", "b", "c"}, ss) + }, fx.ParamTags(`group:"t"`))), + ) + defer app.RequireStart().RequireStop() + }) } func TestReplaceFailure(t *testing.T) { @@ -104,39 +142,44 @@ func TestReplaceFailure(t *testing.T) { assert.Contains(t, err.Error(), "*fx_test.A already decorated") }) - t.Run("replace with annotate", func(t *testing.T) { + t.Run("replace a value that wasn't provided", func(t *testing.T) { t.Parallel() - type A struct { - Value string - } + type A struct{} - app := fxtest.New(t, - fx.Supply( - fx.Annotate(A{"A"}, fx.ResultTags(`name:"t"`)), - ), - fx.Replace( - fx.Annotate(A{"B"}, fx.ResultTags(`name:"t"`)), - ), - fx.Invoke(fx.Annotate(func(a A) { - assert.Equal(t, a.Value, "B") - }, fx.ParamTags(`name:"t"`))), + app := NewForTest(t, + fx.Replace(A{}), + fx.Invoke(func(a *A) { + }), ) - defer app.RequireStart().RequireStop() + err := app.Err() + assert.Error(t, err) + assert.Contains(t, err.Error(), "missing type: *fx_test.A") }) - t.Run("replace a value group with annotate", func(t *testing.T) { + t.Run("replace panics on invalid values", func(t *testing.T) { t.Parallel() - app := fxtest.New(t, - fx.Supply( - fx.Annotate([]string{"A", "B", "C"}, fx.ResultTags(`group:"t"`)), - ), - fx.Replace(fx.Annotate([]string{"a", "b", "c"}, fx.ResultTags(`group:"t"`))), - fx.Invoke(fx.Annotate(func(ss ...string) { - assert.ElementsMatch(t, []string{"a", "b", "c"}, ss) - }, fx.ParamTags(`group:"t"`))), + type A struct{} + type B struct{} + + require.PanicsWithValuef( + t, + "untyped nil passed to fx.Replace", + func() { fx.Replace(A{}, nil) }, + "a naked nil should panic", ) - defer app.RequireStart().RequireStop() + + require.PanicsWithValuef( + t, + "error value passed to fx.Replace", + func() { fx.Replace(A{}, errors.New("some error")) }, + "a naked nil should panic", + ) + + require.NotPanicsf( + t, + func() { fx.Replace(A{}, (*B)(nil)) }, + "a wrapped nil should not panic") }) } From 923c0e8aea8eca41dedec258e81997b58f7b4a6a Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Mon, 14 Feb 2022 10:56:08 -0800 Subject: [PATCH 4/4] address code review --- replace.go | 2 +- replace_test.go | 26 ++++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/replace.go b/replace.go index 6d3b4d1a6..f7fa865cb 100644 --- a/replace.go +++ b/replace.go @@ -30,7 +30,7 @@ import ( // Replace provides instantiated values for graph modification. Similar to // what fx.Supply is to fx.Provide, values provided by fx.Replace behaves -// similar to values produced by decorators specified with fx.Decorate. +// similarly to values produced by decorators specified with fx.Decorate. // // Refer to the documentation on fx.Decorate to see how graph modifications // work with fx.Module. diff --git a/replace_test.go b/replace_test.go index e74c27236..98cec06f3 100644 --- a/replace_test.go +++ b/replace_test.go @@ -104,7 +104,7 @@ func TestReplaceSuccess(t *testing.T) { app := fxtest.New(t, fx.Supply( - fx.Annotate([]string{"A", "B", "C"}, fx.ResultTags(`group:"t"`)), + fx.Annotate([]string{"A", "B", "C"}, fx.ResultTags(`group:"t,flatten"`)), ), fx.Replace(fx.Annotate([]string{"a", "b", "c"}, fx.ResultTags(`group:"t"`))), fx.Invoke(fx.Annotate(func(ss ...string) { @@ -113,6 +113,28 @@ func TestReplaceSuccess(t *testing.T) { ) defer app.RequireStart().RequireStop() }) + + t.Run("replace a value group supplied by a child module from root module", func(t *testing.T) { + t.Parallel() + + foo := fx.Module("foo", + fx.Supply( + fx.Annotate([]string{"a", "b", "c"}, fx.ResultTags(`group:"t,flatten"`)), + ), + ) + + fx.New( + fx.Module("wrapfoo", + foo, + fx.Replace( + fx.Annotate([]string{"d", "e", "f"}, fx.ResultTags(`group:"t"`)), + ), + fx.Invoke(fx.Annotate(func(ss []string) { + assert.ElementsMatch(t, []string{"d", "e", "f"}, ss) + }, fx.ParamTags(`group:"t"`))), + ), + ) + }) } func TestReplaceFailure(t *testing.T) { @@ -174,7 +196,7 @@ func TestReplaceFailure(t *testing.T) { t, "error value passed to fx.Replace", func() { fx.Replace(A{}, errors.New("some error")) }, - "a naked nil should panic", + "replacing with an error should panic", ) require.NotPanicsf(