Skip to content

Commit

Permalink
Build value group values using their original scope
Browse files Browse the repository at this point in the history
We call simple providers with their original scope.
(Ref: https://github.com/uber-go/dig/blob/master/param.go#L287)
This allows decorators to be applied even if the constructor is exported
via `dig.Export(true)`.

However, we call value group value providers with whatever scope we found them in:
(Ref: https://github.com/uber-go/dig/blob/master/param.go#L609)

This has the consequence of causing exported value group value providers
within a module to not be able to be decorated by decorators
within the same module, when using `dig.Export(true)`, unlike
their simple provider counterpart.
For example (playground link: https://go.dev/play/p/R2uqaTvT57P):

```go
type Foo struct{}
type FooResults struct {
	dig.Out

	Foo Foo `group:"foos"`
}

func NewFoo(s string) FooResults {
	fmt.Printf("String in NewFoo: %q\n", s)
	return FooResults{
		Foo: Foo{},
	}
}

type Bar struct{}

func NewBar(s string) Bar {
	fmt.Printf("String in NewBar: %q\n", s)
	return Bar{}
}

type UseFooAndBarParams struct {
	dig.In

	Foos []Foo `group:"foos"`
	Bar  Bar
}

func UseFooAndBar(UseFooAndBarParams) {}

func main() {
	c := dig.New()
	c.Provide(func() string { return "base" })
	child := c.Scope("child")
	child.Decorate(func(s string) string {
		return s + "-decorated"
	})
	child.Provide(NewFoo, dig.Export(true))
	child.Provide(NewBar, dig.Export(true))
}

// Output:
// String in NewFoo: "base"
// String in NewBar: "base-decorated"
```

Since we use `dig.Export(true)` by default in Fx, this is the default
behavior for value group providers, see uber-go/fx#1104

This commit changes `callGroupProviders` to use the value group provider's
original scope as well, and adds a test to verify the behavior is fixed.
  • Loading branch information
JacobOaks committed Aug 2, 2023
1 parent a30081d commit cc4ce88
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
45 changes: 45 additions & 0 deletions dig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,51 @@ import (
"go.uber.org/dig/internal/digtest"
)

func TestValueGroupValuesGetProperlyDecorated(t *testing.T) {
type Foo struct{}
type FooResults struct {
dig.Out

Foo Foo `group:"foos"`
}
type Bar struct{}
type UseBarAndFooParams struct {
dig.In

Foos []Foo `group:"foos"`
Bar Bar
}

var FooCtrRun, BarCtrRun bool
c := digtest.New(t)
c.RequireProvide(func() string { return "base" })

child := c.Scope("child")
child.RequireDecorate(func(s string) string {
return s + "-decorated"
})
child.RequireProvide(func(s string) FooResults {
FooCtrRun = true
// Previously, this would have just been "base",
// because we weren't looking in the constructor's
// original scope for decorators to apply
// when the value is a value group value.
assert.Equal(t, "base-decorated", s)
return FooResults{
Foo: Foo{},
}
}, dig.Export(true))
child.RequireProvide(func(s string) Bar {
BarCtrRun = true
assert.Equal(t, "base-decorated", s)
return Bar{}
}, dig.Export(true))
child.RequireInvoke(func(UseBarAndFooParams) {})

assert.True(t, FooCtrRun)
assert.True(t, BarCtrRun)
}

func TestEndToEndSuccess(t *testing.T) {
t.Parallel()

Expand Down
2 changes: 1 addition & 1 deletion param.go
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ func (pt paramGroupedSlice) callGroupProviders(c containerStore) (int, error) {
providers := c.getGroupProviders(pt.Group, pt.Type.Elem())
itemCount += len(providers)
for _, n := range providers {
if err := n.Call(c); err != nil {
if err := n.Call(n.OrigScope()); err != nil {
return 0, errParamGroupFailed{
CtorID: n.ID(),
Key: key{group: pt.Group, t: pt.Type.Elem()},
Expand Down

0 comments on commit cc4ce88

Please sign in to comment.