Dependency Injection: register IGrainFactory

[ReubenBond]

Currently I cannot construct objects which require IGrainFactory using DI in Orleans because the implementation is not registered.

Example of a failure scenario:

public HasDeps : Grain, IHasDeps
{
  public HasDeps(SomeDep someDep) { /* ... */ }
}

// Imagine I register SomeDep in the container...
public SomeDep
{
  public SomeDep(IGrainFactory grainFactory) { /* ... */ }
}

Is there a good reason for IGrainFactory (and friends?) not to be in the container?

[nehmebilal]

Can you register the IGrainFactory yourself the same way you register any other dependency?

I think the long term plan is to potentially have multiple instances of the IGrainFactory (client connecting to multiple silos or silo connecting to another silo, see #467) in which case you'll need to use named dependencies or to inject some sort of service locator that allows you to retrieve the appropriate GrainFactory inside SomeDep.

[ReubenBond]

This is on the silo, not the client. The IGrainFactory which is injected into SomeDep should be the same as the one injected into HasDeps.

[nehmebilal]

How do you know? If the Silo is connecting to another Silo, there will be another instance of IGrainFactory that is associated with the second Silo. SomeDep is just a class and there is no way to tell if it's looking for currentSilo.GrainFactory or OtherSilo.GrainFactory.

[ReubenBond]

It should take the instance from the current lifetime/scope, right? It would be weird if the IGrainFactory in HasDeps differs from that in SomeDep without me explicitly saying it should differ.

How do you propose we actually provide IGrainFactory to dependency classes?

[jason-bragg]

"Can you register the IGrainFactory yourself "

Not currently, its implementation is internal.

"If the Silo is connecting to another Silo, there will be another instance of IGrainFactory that is associated with the second Silo"

??? This breaks the virtual actor model, does it not ??? I don't want to side track this discussion, but this statement seems to be counter to the fundamentals of the programing model.

At tertiary glance it looks like we can add the grain factory to the list of services (IServiceCollection) in RegisterSystemTypes(). We'll need to get it from the service provider when setting it in the runtime, or removed it from the runtime and always get it from the service provider (and other such cleanup). Does not look hard...?

[nehmebilal]

??? This breaks the virtual actor model, does it not ??? I don't want to side track this discussion, but this statement seems to be counter to the fundamentals of the programing model.

That's a good point, I don't know, up to you guys to decide :smiley: You still have the issue on the client side if you're trying to connect to multiple silos.

I am not sure what DI framework is Orleans using right now, but here is how I would do it with Unity:

container.RegisterType<IGrainFactory>("Silo1Factory", new ContainerControlledLifetimeManager(), new InjectionFactory(
    c =>
    {
        GrainClient.Initialize("OrleansClientConfiguration1.xml");
        return GrainClient.GrainFactory;
    }));

container.RegisterType<IGrainFactory>("Silo2Factory", new ContainerControlledLifetimeManager(), new InjectionFactory(
    c =>
    {
        GrainClient.Initialize("OrleansClientConfiguration2.xml");
        return GrainClient.GrainFactory;
    }));            

container.RegisterType<SomeDep>(
    new InjectionConstructor(                        
        new ResolvedParameter<IRepository>("Silo1Factory")
    )
);

Sorry that doesn't help for the server side GrainFactory which I am not sure how to retrieve.

[ReubenBond]

If the Silo is connecting to another Silo, there will be another instance of IGrainFactory that is associated with the second Silo

??? This breaks the virtual actor model, does it not ??? I don't want to side track this discussion, but this statement seems to be counter to the fundamentals of the programing model.

I believe @nehmebilal meant to say "cluster" instead of "silo"

[nehmebilal]

I believe @nehmebilal meant to say "cluster" instead of "silo"

:+1:

[sergeybykov]

We definitely want a client to be able to connect to multiple clusters. However, I do not see a case for needing the same on the silo side. Mocking for testing is a different question, of course.

[nehmebilal]

Then we just need to register the IGrainFactory as part of Silo initialization as suggested by @ReubenBond and @jason-bragg. That should solve the server side dependency injection of IGrainFactory.

[vidrir]

I would love to see this resolved. We need a way to pass the IGrainFactory when constructing our Mocks for DI in testing.

[michaelahern]

+1

[jason-bragg]

I took a pass at this in "Injection of grain factory #1342"

[jason-bragg]

I withdrew PR #1342". My first pass at this simply registered the GrainFactory with the set of Orleans services provided to the container during it's initialization. This allows for users to replace this implementation, but this does not really work because, while GrainFactory conforms to a public interface IGrainFactory, Orleans internals relies on internal calls in the concrete GrainFactory implementation. These call's can't be mimicked by an external class, so the GrainFactory can't be injected in any practical manner.

This is not, as i'd initially assumed, simply a case of registering the grain factory with the DI system and requesting it from the service provider when needed.

To do this, we'll need to separate the public GrainFactory capabilities (described in IGrainFactory) from the internal capabilities, which is more than I was willing to take on at this time.

[xiazen]

so step 1: separate public capabilities in concrete GrainFactory implementation to a new class PublicGrainFacotry, clean original GrainFactory class to reduce code redundancy. step 2: figure out DI on public GrainFactory.

I can take the initiate to look at step 1, and then step 2.

[jason-bragg]

"separate public capabilities in concrete GrainFactory implementation to a new class PublicGrainFactory"

I don't think we want a PublicGrainFactory class as much as we want the Interface to contain all of the publicly necessary calls. Am I missing something? Is there a need for a public grain factory concrete class?

"Figure out DI on public GrainFactory"

Agreed, but I think we want to register the grain factory by interface rather than concrete type.

Step 3: Profit!

[xiazen]

addressed comments in person

[jdom]

Resolved by #2192