Skip to content

Commit

Permalink
Fix decorartion logic for paramSingle
Browse files Browse the repository at this point in the history
When building paramSingle using decorators, there was a descrepancy when
the invoking Scope does not have a decorator for the type but one of its
parent Scopes does. This resulted in the parent Scope having to Invoke
a function that uses the decorated type for the child to start seeing it
too.

This fixes paramSingle to search all effective scopes for a decorator
and invoke that instead of searching just the scope it was invoked from.

Fixes uber-go#316
  • Loading branch information
sywhang committed Feb 6, 2022
1 parent 1d9f0f1 commit 27af7c8
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 26 deletions.
83 changes: 61 additions & 22 deletions decorate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,56 +34,95 @@ func TestDecorateSuccess(t *testing.T) {
t.Run("simple decorate without names or groups", func(t *testing.T) {
t.Parallel()
type A struct {
name string
Name string
}

c := digtest.New(t)
c.RequireProvide(func() *A { return &A{name: "A"} })
c.RequireProvide(func() *A { return &A{Name: "A"} })

c.RequireInvoke(func(a *A) {
assert.Equal(t, "A", a.name, "expected name to not be decorated yet.")
assert.Equal(t, "A", a.Name, "expected name to not be decorated yet.")
})

c.RequireDecorate(func(a *A) *A { return &A{name: a.name + "'"} })
c.RequireDecorate(func(a *A) *A { return &A{Name: a.Name + "'"} })

c.RequireInvoke(func(a *A) {
assert.Equal(t, "A'", a.name, "expected name to equal decorated name.")
assert.Equal(t, "A'", a.Name, "expected name to equal decorated name.")
})
})

t.Run("simple decorate a provider from child scope", func(t *testing.T) {
t.Parallel()
type A struct {
name string
Name string
}

c := digtest.New(t)
child := c.Scope("child")
child.RequireProvide(func() *A { return &A{name: "A"} }, dig.Export(true))
child.RequireProvide(func() *A { return &A{Name: "A"} }, dig.Export(true))

child.RequireDecorate(func(a *A) *A { return &A{name: a.name + "'"} })
child.RequireDecorate(func(a *A) *A { return &A{Name: a.Name + "'"} })
c.RequireInvoke(func(a *A) {
assert.Equal(t, "A", a.name, "expected name to equal original name in parent scope")
assert.Equal(t, "A", a.Name, "expected name to equal original name in parent scope")
})

child.RequireInvoke(func(a *A) {
assert.Equal(t, "A'", a.name, "expected name to equal decorated name in child scope")
assert.Equal(t, "A'", a.Name, "expected name to equal decorated name in child scope")
})
})

t.Run("check parent-provided decorator doesn't need parent to invoke", func(t *testing.T) {
type A struct {
Name string
}

type B struct {
dig.In

Values []string `group:"values"`
}
type C struct {
dig.Out

Values []string `group:"values"`
}

c := digtest.New(t)
child := c.Scope("child")

c.RequireProvide(func() *A { return &A{Name: "A"} }, dig.Export(true))
c.RequireProvide(func() string { return "val1" }, dig.Export(true), dig.Group("values"))
c.RequireProvide(func() string { return "val2" }, dig.Export(true), dig.Group("values"))
c.RequireProvide(func() string { return "val3" }, dig.Export(true), dig.Group("values"))
c.RequireDecorate(func(a *A) *A { return &A{Name: a.Name + "'"} })
c.RequireDecorate(func(b B) C {
var val []string
for _, v := range b.Values {
val = append(val, v+"'")
}
return C{
Values: val,
}
})
child.RequireInvoke(func(a *A, b B) {
assert.Equal(t, "A'", a.Name, "expected name to equal decorated name in child scope")
assert.ElementsMatch(t, []string{"val1'", "val2'", "val3'"}, b.Values)
})
})

t.Run("simple decorate a provider to a scope and its descendants", func(t *testing.T) {
t.Parallel()
type A struct {
name string
Name string
}

c := digtest.New(t)
child := c.Scope("child")
c.RequireProvide(func() *A { return &A{name: "A"} })
c.RequireProvide(func() *A { return &A{Name: "A"} })

c.RequireDecorate(func(a *A) *A { return &A{name: a.name + "'"} })
c.RequireDecorate(func(a *A) *A { return &A{Name: a.Name + "'"} })
assertDecoratedName := func(a *A) {
assert.Equal(t, a.name, "A'", "expected name to equal decorated name")
assert.Equal(t, a.Name, "A'", "expected name to equal decorated name")
}
c.RequireInvoke(assertDecoratedName)
child.RequireInvoke(assertDecoratedName)
Expand All @@ -92,31 +131,31 @@ func TestDecorateSuccess(t *testing.T) {
t.Run("modifications compose with descendants", func(t *testing.T) {
t.Parallel()
type A struct {
name string
Name string
}

c := digtest.New(t)
child := c.Scope("child")
c.RequireProvide(func() *A { return &A{name: "A"} })
c.RequireProvide(func() *A { return &A{Name: "A"} })

c.RequireDecorate(func(a *A) *A { return &A{name: a.name + "'"} })
child.RequireDecorate(func(a *A) *A { return &A{name: a.name + "'"} })
c.RequireDecorate(func(a *A) *A { return &A{Name: a.Name + "'"} })
child.RequireDecorate(func(a *A) *A { return &A{Name: a.Name + "'"} })

c.RequireInvoke(func(a *A) {
assert.Equal(t, "A'", a.name, "expected decorated name in parent")
assert.Equal(t, "A'", a.Name, "expected decorated name in parent")
})

child.RequireInvoke(func(a *A) {
assert.Equal(t, "A''", a.name, "expected double-decorated name in child")
assert.Equal(t, "A''", a.Name, "expected double-decorated name in child")
})

sibling := c.Scope("sibling")
grandchild := child.Scope("grandchild")
require.NoError(t, sibling.Invoke(func(a *A) {
assert.Equal(t, "A'", a.name, "expected single-decorated name in sibling")
assert.Equal(t, "A'", a.Name, "expected single-decorated name in sibling")
}))
require.NoError(t, grandchild.Invoke(func(a *A) {
assert.Equal(t, "A''", a.name, "expected double-decorated name in grandchild")
assert.Equal(t, "A''", a.Name, "expected double-decorated name in grandchild")
}))
})

Expand Down
17 changes: 13 additions & 4 deletions param.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,19 @@ func (ps paramSingle) getValue(c containerStore) (reflect.Value, bool) {
return _noValue, false
}

// builds the parameter using decorators, if any. If there are no decorators associated
// with this parameter, _noValue is returned.
// builds the parameter using decorators in all scopes that affect the
// current scope, if there are any. If there are multiple Scopes that decorates
// this parameter, the closest one to the Scope that invoked this will be used.
// If there are no decorators associated with this parameter, _noValue is returned.
func (ps paramSingle) buildWithDecorators(c containerStore) (v reflect.Value, found bool, err error) {
decorators := c.getValueDecorators(ps.Name, ps.Type)
var decorators []decorator
var decoratingScope containerStore
for _, s := range c.storesToRoot() {
if decorators = s.getValueDecorators(ps.Name, ps.Type); len(decorators) > 0 {
decoratingScope = s
break
}
}
if len(decorators) == 0 {
return _noValue, false, nil
}
Expand All @@ -240,7 +249,7 @@ func (ps paramSingle) buildWithDecorators(c containerStore) (v reflect.Value, fo
}
return v, found, err
}
v, _ = c.getDecoratedValue(ps.Name, ps.Type)
v, _ = decoratingScope.getDecoratedValue(ps.Name, ps.Type)
return
}

Expand Down

0 comments on commit 27af7c8

Please sign in to comment.