defval / di

🛠 A full-featured dependency injection container for go programming language.
MIT License
232 stars 13 forks source link

Add support for di. Inject in Invoke() #10

Closed erickskrauch closed 4 years ago

erickskrauch commented 4 years ago

Actually, the title. I've prepared an example that shows the problem: https://play.golang.org/p/vZ2JvfmvJBB

package main

import (
    "log"

    "github.com/goava/di"
)

type Dependency struct {}

func NewDependency() *Dependency {
    return &Dependency{}
}

type Params struct {
    di.Inject

    Dependency *Dependency `di:""`
}

func InvokeWithDependencyAsParam(params Params) {
    // Some code
}

func main() {
    _, err := di.New(
        di.Provide(NewDependency),
        di.Invoke(InvokeWithDependencyAsParam),
    )
    if err != nil {
        log.Fatal(err)
    }
}
defval commented 4 years ago

Yes, you are. It is not supported in Invoke(). How critical is this for you? I can make a quick fix. But ideally, I would like to merge this into a single release with #9. This mechanism can be made better at the internal implementation level.

defval commented 4 years ago

I think it will be ready by Sunday evening.

defval commented 4 years ago

Ou. This is already working in the #9 =D

https://github.com/goava/di/pull/9/commits/fae7fe988a5ffa87454c46d3b5db0c3e54a5b5aa

erickskrauch commented 4 years ago

Thank you very much for such a quick response! It's nice to see a responsible maintainer :)

I use Invoke to initialize subscribers to dispatcher events. As a temporary solution, I began to receive a Container that I threw into the dependency graph. But unfortunately, I now get a bug about changing the dependency graph after compiling it, which is weird.

Container creation: https://github.com/elyby/chrly/blob/di/di/di.go

Module creation (extended https://github.com/elyby/chrly/blob/di/di/dispatcher.go):

package di

import (
    "github.com/goava/di"
    "github.com/mono83/slf"

    dispatcherModule "github.com/elyby/chrly/dispatcher"
    "github.com/elyby/chrly/eventsubscribers"
    "github.com/elyby/chrly/http"
    "github.com/elyby/chrly/mojangtextures"
)

var dispatcher = di.Options(
    di.Provide(newDispatcher,
        di.As(new(http.Emitter)),
        di.As(new(mojangtextures.Emitter)),
    ),
    di.Invoke(enableEventsHandlers),
)

func newDispatcher() dispatcherModule.EventDispatcher {
    return dispatcherModule.New()
}

func enableEventsHandlers(
    container *di.Container,
    dispatcher dispatcherModule.EventDispatcher,
    logger slf.Logger,
) error {
    (&eventsubscribers.Logger{Logger: logger}).ConfigureWithDispatcher(dispatcher)

    var statsReporter slf.StatsReporter
    err := container.Provide(&statsReporter)
    if err != nil {
        return err
    }

    if statsReporter != nil {
        (&eventsubscribers.StatsReporter{StatsReporter: statsReporter}).ConfigureWithDispatcher(dispatcher)
    }

    return nil
}

The error I get:

di.Invoke(..) failed: /home/erickskrauch/golang/src/github.com/elyby/chrly/di/dispatcher.go:18: dependency providing restricted after container compile
erickskrauch commented 4 years ago

My bad. I used container.Provide instead of container.Resolve. Everything works :)

defval commented 4 years ago

Thank you very much for such a quick response! It's nice to see a responsible maintainer :)

No problem. The quarantine is to blame =D

Can you try the master branch with new features? I'm merging #9 for release.

erickskrauch commented 4 years ago

Sure, I will update right now and give you feedback.

erickskrauch commented 4 years ago

It works perfectly: image

Nothing is broken, I just removed di.WithCompile() and container.Compile() calls and that's all migration :)

defval commented 4 years ago

@erickskrauch I read your screenshot. You can use di.As() for process all type with ConfigureWithDispatcher method:

Define an interface for event subscribers:

type EventSubscriber interface {
    ConfigureWithDispatcher(dispatcher EventDispatcher)
}

Provide implementation as EventSubscriber:

di.Provide(eventsubscribers.NewLogger, di.As(new(EventSubscriber))),
di.Provide(eventsubscribers.NewStatsReporter, di.As(new(EventSubscriber))),

And process all interfaces together:

func Configure(dispatcher EventDispatcher, subscribers []EventSubscriber) {
    for _, sub := range subscribers {
        sub.ConfigureWithDispatcher(dispatcher)
    }
}
container.Invoke(Configure)
defval commented 4 years ago

Or modify EventDispatcher constructor:

func NewEventDispatcher(subscribers []EventSubscriber) *EventDispatcher {
    d := &EventDispatcher{}
    for _, sub := range subscribers {
        sub.ConfigureWithDispatcher(d)
    }
    return d
}
erickskrauch commented 4 years ago

Amazing, thanks for the suggestion!