Populate a map by combining name and group attributes
jquirke opened this issue · 7 comments
Currently FX supports injecting multiple different implementations of an interface type using groups
https://pkg.go.dev/go.uber.org/fx#hdr-Value_Groups
This allows me to Provide multiple implementations of an interface Foo provided they are annotated with group, and collect them
type Params struct {
fx.In
Foos []Foo `group:"myfoos"`
where a particular Foo can be provided as
type Out struct {
fx.Out
Foo `group:"myfoos"`
However, the Foos I received back are not able to be identified by name.
Describe the solution you'd like
I would like to be able to populate a map of named foos, using the existing name annnotation:
type Params struct {
fx.In
Foos map[string]Foo `group:"myfoos"`
where a particular Foo can be provided as
type Out struct {
fx.Out
Foo1 Foo `name:"foo1" group:"myfoos"`
Foo2 Foo `name:"foo2" group:"myfoos"`
This allows me a powerful capability: To name and group a type together, and obtain it as an unordered slice, a single instance by name, or as a map of names to instances.
Describe alternatives you've considered
A trivial solution is to extend Foo to include a Name() method, and then simply organise this into a map[string]Foo.
foos := make(map[string]Foo)
for _, foo := range params.Foos {
foos[foo.Name()] = foo
}
But there are many cases where components need to be looked up by name according to dynamic configuration, that re-implementing this for each case seems inelegant and would be nice to delegate to the DI framework.
Is this a breaking change?
It is difficult to imagine a scenario where this would cause a breaking change here; but of course, there is also the curse of hindsight
- Currently attaching a group attribute to a dependency type of map fails and the application would not start, therefore this cannot be a breaking change in a practical sense.
bad argument 1: bad field "Foo" of handler.Params: value groups may be consumed as slices only: field "Foo" (map[string]string) is not a slice
- Annotating a provided dependency with both a name and a group similarly fails:
bad result 1: bad field "Foo" of handler.Result: cannot use named values with value groups: name:"foo1" provided with group:"myfoos"
Internal Ref: GO-1864
Some thoughts on the behaviour:
Consider:
type Out struct {
fx.Out
Foo Foo `name:"foo1" group:"foos"`
Foo2 Foo `group:"foos2"`
Foo3 Foo `name:"foo3"`
}
type Params struct {
fx.In
MapOfFoos map[string]Foo `group:"foos"`
GroupOfFoos []Foo `name:"foo1" group:"foos"`
GroupOfFoos2 []Foo `group:"foos2"`
SingleFoo Foo `name":foo1"`
SingleFoo3 Foo `name":foo3"`
}
Before this proposed change, this will fail at runtime for at least 2 reasons as listed in the summary above.
But if we implement the logic, what should the behaviour be?
Should a dig output with both a name and a group be treated as special and only go in maps to maximise backward compatibility?
Should we avoid allowing a name or group to be re-used individually?
Example Behaviour 1 output (name+group, name, group all allowed - keys are decomposable)
MapOfFoos = map[string]Foo{"foo1": Foo}
GroupOfFoos = []{Foo}
GroupOfFoos2 = []{Foo2}
SingleFoo = Foo1
SingleFoo = Foo3
Example Behaviour 2 output (failure at Invoke time: a dig key is now treated as name+group){}
MapOfFoos = map[string]Foo{"foo1": Foo}
GroupOfFoos = <cause failure>
GroupOfFoos2 = []{Foo2}
SingleFoo1 = <cause failure>
SingleFoo3 = Foo3
Example Behaviour 2b (failure: a dig key is now treated as name+group, but both constituent components cannot be unique elsewhere in the container. Fail at Provide time).
e.g.
type Out struct {
fx.Out
Foo Foo `name:"foo" group:"foos"`
Foo2 Foo `group:"foos"` // illegal, runtime failure at provide time - foos was declared as the group in a name+group key already
Foo3 Foo `name:"foo"` // illegal, runtime failure at provide time: foo was declared in a name+group already
}
Other considerations:
Should this new mode be specifically and explicitly enabled via a dig Container or Scope Option? If so, at which level? Should this be exposed to Fx? If so, how?
What is the current status of this?
I also have the following scenario where this would be really helpful:
package main
import (
"context"
"fmt"
"go.uber.org/fx"
)
type Database struct {
Name string
}
type InputParams struct {
fx.In
Db1 *Database `name:"db1"`
Db2 *Database `name:"db2"`
}
type InputHookParams struct {
fx.In
Connections []Database `group:"sql_connections"`
}
type Databases struct {
fx.Out
Db1 *Database `name:"db1" group:"sql_connections"`
Db2 *Database `name:"db2" group:"sql_connections"`
}
func SetupDatabases() (Databases, error) {
return Databases{
Db1: &Database{
Name: "user",
},
Db2: &Database{
Name: "cart",
},
}, nil
}
func RunExample(p InputParams) {
fmt.Println("1: " + p.Db1.Name)
fmt.Println("2: " + p.Db2.Name)
}
func RegisterOnStopHook(lc fx.Lifecycle, conns InputHookParams) {
lc.Append(fx.Hook{
OnStop: func(ctx context.Context) error {
for _, c := range conns.Connections {
fmt.Println("closing connection with " + c.Name + " ...")
}
return nil
},
})
}
func main() {
fx.New(
fx.Provide(SetupDatabases),
fx.Invoke(RegisterOnStopHook),
fx.Invoke(RunExample),
).Run()
}I want to inject multiple instances of *Database and each one will have specific configurations. That's why it is important to name them. Also, I need all of them to register a OnStop hook (which could be easily achieved by using the group tag).
Is there another way to achieve that?
I had this same issue on January and by the time after reading the docs and other issues, I thought that my use case was a bad implementation of Fx, now that I'm more experienced with the Framework I guess it wasn't.
I was going to write an Issue regarding this problem when I found this that matches my exact use case, is there any update on this @jquirke? Is there a reason why the PR haven't been approved and merged yet? Did you get to a dead end? May I help with something?
Thank you @jquirke for all the work you've done, it really was the exact thing I was looking for!
I was going to write an Issue regarding this problem when I found this that matches my exact use case, is there any update on this @jquirke? Is there a reason why the PR haven't been approved and merged yet? Did you get to a dead end? May I help with something?
No dead end, it's just there are a ton of corner cases to cover and test and it requires coordination with the Fx Layer as well
For the problem I was trying to solve within Uber there were other workable options, and it's something I've just let slip down the priority list.
Feel free to take what I've done so far and run with it; I may try and finish this given the demand for the feature both in public and inside Uber but unfortunately I can't make any promises.