dotnet / orleans

Cloud Native application framework for .NET
https://docs.microsoft.com/dotnet/orleans
MIT License
10.07k stars 2.03k forks source link

Support for dependency injection #39

Closed sergeybykov closed 9 years ago

jason-bragg commented 9 years ago

I want to touch on this issue as DI/IC (I’ll just call it DI from now on) can help resolve some of the other issues under discussion, especially the question of testability.

Everyone loves the idea of DI, but, from my experience, when digging into the realities of actually employing it in a product, opinions rapidly diverge. Part of the reason for this is that DI is quite complicated. There are entire projects dedicated to this problem. A simple DI implementation can provide developers much of what they want, but use requires clear understanding of the limitations and acceptance of its constraints (sometimes hard to accept when there are shinier implementations available). More complete implementations require more of a learning curve, and in many cases, still have constraints and nuances. Additionally, Orleans is a framework technology, meaning that it will be less attractive if we utilize a DI system different from our consumers.

With the above in mind, I suggest the following: 1) Identify and agree upon a minimal set of DI functionality with a well-known set of constraints. 2) Develop a clear, well defined, and well documented internal interface for this set of functionality. 3) Develop a set of functional tests around the expected behaviors and constraints. 4) Develop a default implementation. 5) Update Orleans framework to utilize the limited DI functionality. This should vet the interface and default implementation. 6) Make the limited DI interface public, and expose this interface through the runtime in a way that allows application developers to replace the default implementation with their own. 7) Work with customers and other open source DI framework developers to develop (or encourage the development of) implementations for commonly used DI frameworks.

This would allow the Orleans framework to utilize a limited set of DI capabilities while allowing Orleans customers to integrate their choice of DI solution into Orleans, and be able to take advantage of the full capabilities of their selected solution in their grain code.

jthelin commented 9 years ago

Would this meet some or all of those requirements, @jason-bragg ? https://github.com/aspnet/DependencyInjection

That seems to be the way other .NET projects are heading on integration with different DI frameworks.

yevhen commented 9 years ago

I propose to not over-engineer it. Dependency injection is just this:

Func<Type, object> activator = type => Activator.CreateInstance(type);

Just giving developers ability to set this delegate is more than enough. Than, they can easily plug any of their favorite DI container libraries.

That will make everyone happy and without taking any dependencies on 3rd party lib. Even, if it was created by Microsoft :)

Less is more.

jason-bragg commented 9 years ago

@jthelin-microsoft At first glance it seems like a good fit. It looks a bit more involved than I was thinking, but as I mentioned, the subject in general is deceptively complex, and I doubt I've spent as much time pondering an abstraction layer as the DependencyInjection developers.

Do you have any experience with this tech? Do you know anyone who is using it?

wickedshimmy commented 9 years ago

I highly recommend this read if you're thinking about designs (which has a nice discussion of the good/bad patterns in the ASP.NET stack): http://blog.ploeh.dk/2014/05/19/di-friendly-framework/

MovGP0 commented 9 years ago

Dependency Injection might not be possible with the current generated code. The issue is that the grains are instantiated by static factories, which are not replaceable with a mock:

public class PlayerGrain : GrainBase, IPlayerGrain
{
    Task JoinGame(Guid gameId)
    {
        // generated static factory can't be injected
        var game = await GameGrainFactory.GetGrain(gameId);

        // ...
    }
}

What the code generator should generate instead is a non-static factory with an appropriate interface:

public class PlayerGrain : GrainBase, IPlayerGrain
{
    private IGrainFactory<IGameGrain> GameGrainFactory { get; set; }

    public PlayerGrain(IGrainFactory<IGameGrain> gameGrainFactory)
    {
        GameGrainFactory = gameGrainFactory;
    }

    Task JoinGame(Guid gameId)
    {
        // injected factory
        var game = await GameGrainFactory.GetGrain(gameId);

        // ...
    }
}

To make the constructor injection possible, the framework might need to bring its own high-performance DI library, or a generic wrapper for DI containers as ASP.NET vNext does.

I'd recomend taking a look at DryIoc, which is the best performing framework I know:

nothingmn commented 9 years ago

I personally would love to see the full/complete feature set from the asp.net team's DI (https://github.com/aspnet/DependencyInjection) project in place; which enables us to BYOContainer; not to mention the time savings. Along with that, an upgrade to Orlean's codegen to, by default, leverage the DI abstraction/framework.

Cheers,

-Rob

ReubenBond commented 9 years ago

People will balk at this, but how about just exposing a Func<Type, Grain> property to be used in place of Activator.CreateInstance when instantiating grains in Category for now?

MovGP0 commented 9 years ago

When you use Func<Type, Grain>, then you loose the ability to use something other than Type to get an instance of Grain. I guess it should be more something like:

public sealed class GameGrain : GrainBase, IGameGrain
{
    private IActivator Activator { get; set; } // might be part of GrainBase

    // Dependency injection of Activator
    public GameGrain(IActivator activator /*...*/)
    {
        Activator = activator;
        // ...
    }

    public Task Foo()
    {
        // Using different ways to instantiate 
        var player1 = Activator.CreateInstance<IPlayerGrain>();
        var player2 = Activator.CreateInstance<IPlayerGrain>(Guid.NewGuid());
        // ...
    }
}
ReubenBond commented 9 years ago

@MovGP0 I'm talking about for the construction of the grain itself - you could always just inject IServiceProvider/container-of-choice into your grain, or IGrainFactory<IPlayerGrain> in your case

yevhen commented 9 years ago

Well, if more ppl realize that this is the leanest approach we can get, then we might eventually arrive at something ;)

ReubenBond commented 9 years ago

:+1: @yevhen, that's what I meant :)

veikkoeeva commented 9 years ago

+1 To what @yevhen and @ReubenBond suggests. In addition, I believe this is also how the Katana middleware does this, one reason being lambdas are more usable from a F# perspective (so as not to complicate F# story further).

Plasma commented 9 years ago

Agree with @ReubenBond and @yevhen +1

ReubenBond commented 9 years ago

Great 😊 that's at least 3 of us. Maybe we can bring it up after Yevhen's talk at the March 7th meet up.

-----Original Message----- From: "Andrew Armstrong" notifications@github.com Sent: ‎23/‎02/‎2015 22:10 To: "dotnet/orleans" orleans@noreply.github.com Cc: "Reuben Bond" reuben.bond@gmail.com Subject: Re: [orleans] Support for dependency injection (#39)

Agree with @ReubenBond and @yevhen +1 — Reply to this email directly or view it on GitHub.

ReubenBond commented 9 years ago

So where do we stand on this? It sounds like we have enough consensus for someone to implement the simple delegate approach to DI. If I have the time, I may implement it, but someone else please feel free to beat me to it :smile:

sergeybykov commented 9 years ago

@ReubenBond I think we are all in a high-level agreement, but we haven't fully designed the feature. If you can start with something simple, that would be great. We'll be able to iterate on the initial version and finalize all the details in the concrete context and with your use case as a validator.

luisrudge commented 9 years ago

:+1: please add this :)

veikkoeeva commented 9 years ago

Likely of interest is #312.

mcintyre321 commented 9 years ago

@yevhen Not sure your delegate is enough - you may also need to be able to dispose dependencies when you remove the actor. You shouldn't leave it up to the actor, either, as the DI container is meant to be in control of the lifecycle.

Maybe

   Func<Type, Tuple<object, Action>> activator = type => Tuple.Create(Activator.CreateInstance(type), new Action(() => {});

where Item1 is the resolved object and Item2 is the Dispose function

sergeybykov commented 9 years ago

Now that https://github.com/dotnet/orleans/pull/443 has been merged, this issue should be resolved.

luisrudge commented 9 years ago

Awesome. Congratulation guys!