Akka.NET Dependency Injection testkit for #942

[Aaronontheweb]

Do not merge. There will be test failures.

This PR introduces Akka.DI.TestKit, which will be available as a NuGet package, and introduces the following test libraries:

• Akka.DI.Ninject.Tests
• Akka.DI.StructureMap.Tests
• Akka.DI.AutoFac.Tests
• Akka.DI.CastleWindsor.Tests
• Akka.DI.Unity.Tests

Based on what I've been able to recreate locally using my development machine, I've found that our DI system currently leaks injected resources and there's a lot of inconsistencies across DI framework implementations as to whether or not they clean up IDisposable resources from terminated actors.

We'll use the results from the CI server to create a discussion here on what the expected and recommended behavior for DI inside Akka.NET should be.

Some containers may have to use additional options that are specific to the container's configuration in order to achieve these goals. That's fine, but we have to document what those are and explicitly list them on http://getakka.net/ as official project recommendations when working with specific DI libraries.

[Aaronontheweb]

See #942 for the original discussion.

[Aaronontheweb]

Here's the set of tests that we run for each DI framework as part of Akka.DI.TestKit;

• DependencyResolver_should_inject_new_instances_into_DiPerRequestActor - new instances should be injected for each actor.
• DependencyResolver_should_inject_new_instances_on_Restart - new instances should be injected into actors who restart
• DependencyResolver_should_inject_same_instance_into_DiSingletonActor - total anti-pattern with actors when injecting mutable objects, but the goal is to test singleton object lifecycles with actors (managed externally.)
• DependencyResolver_should_inject_instances_into_DiChildActor - test that child actor gets its injections via Context.DI.Props<TActor>()
• DependencyResolver_should_inject_into_normal_mailbox_Actor - control test, used to compare against the results of the stash tests below.
• DependencyResolver_should_inject_into_UnboundedStash_Actor - verify that injection works with Stashing. (since we've had issues with this in the past.)
• DependencyResolver_should_inject_into_BoundedStash_Actor - even though bounded stashing isn't implemented yet.
• DependencyResolver_should_dispose_IDisposable_instances_on_Actor_Termination - DI containers should automatically release resources upon actor termination
• DependencyResolver_should_dispose_IDisposable_instances_on_Actor_Restart - DI containers should automatically release resources upon actor restart

[Aaronontheweb]

So, the discussion.

Akka.DI Behavior Assumptions

These tests make the following assumptions about how IDependencyResolvers are expected to behave in Akka.NET:

1. New resources are always injected into each actor created from the same Props object by default, but the user can specify otherwise in how they configure their DI container.
2. If an actor restarts, new instances of each dependency are created and re-injected into the actor instances. Resources are bound to the lifecycle of a specific ActorBase object - not the lifecycle of the ActorCell or the ActorRef.
3. If an actor restarts or terminates, any IDisposable resources are expected to be released / otherwise managed by the container. The user should not have to manually release those resources themselves inside PostStop if they're managed by a container, but users can still do this if they wish.
4. DI should work with stashing and any other types of actor customizations outside of DI.

Right now, 100% of Akka.DI implementations fail assumption 3. And CastleWindsor fails assumption 1 as well (and, in fact, currently crashes the XUnit2 test runner.)

Decisions that must be made

We have to do both of the following:

1. Decide on whether or not Akka.DI should run based on these assumptions, or different assumptions.
2. Rigorously document how users of the supported DI frameworks need to configure their containers in order to meet these assumptions.

Some of these issues are a matter of just configuring the container "correctly." We need to document what that looks like for each framework once the assumptions are decided upon. So let's start that discussion now: cc @akkadotnet/developers @akkadotnet/contributors

[stefansedich]

Following on from the discussion on gitter. I agree with all of the assumptions listed above, as discussed it seems as though we are not in the case of say Autofac attaching a lifetime scope to the lifetime of the ActorBase.

The hook for disposing components should be on both Restart and Terminate, any new instance of an actor is created within a new scope and any time the actor goes away whether it is a restart or termination we dispose this scope and then create a new scope upon creating the new actor instance.

[Aaronontheweb]

Since CastleWindsor is hanging the entire test runner at the moment, I ran all of the specs individually on my machine and here are what the results for each supported DI framework look like currently:

[Danthar]

I agree with almost all the assumptions outlined by @Aaronontheweb . Except for number 3.

If resources are managed by an container, they should also be disposed by the container. Even though it would be possible to manually dispose them in the PostStop handler, You really should not do that. There is a reason you are letting a container manage those resources. If you manually cleanup resources that are injected by an container you have to be very sure about what you are doing.
It can only lead to bugs. Not all containers can properly track the lifecycle of your resource if you manually dispose of it, where it does not expect you to.

[rogeralsing]

👍 on what @Danthar said.
If the DI framework gave you a disposable resource, you are not the owner of it and you may not dispose it yourself.

[nvivo]

I got a little confused by this:

DependencyResolver_should_inject_new_instances_on_Restart - new instances should be injected into actors who restart

Why does Akka care which instance of a dependency is injected into an actor?

Seems to me this depends only on the scope of the registered dependency. Akka shouldn't expect always a new instance to be injected. It should just accept whatever the container gives to it, and that may vary depending on the defaults of the framework you're using.

[Aaronontheweb]

Seems to me this depends only on the scope of the registered dependency. Akka shouldn't expect always a new instance to be injected. It should just accept whatever the container gives to it, and that may vary depending on the defaults of the framework you're using.

Not referring to DI container lifecycles in this test. In fact, I'm ensuring that they are respected.

What I wanted to avoid is having Props cache the output from DI (I wrote this test without looking at the implementation) and re-use that.

Not all containers can properly track the lifecycle of your resource if you manually dispose of it, where it does not expect you to.

Ok, so it seems like @Danthar and @rogeralsing take issue with this specific part of issue 3

The user should not have to manually release those resources themselves inside PostStop if they're managed by a container, but users can still do this if they wish.

I was considering adding some overridable behavior inside PostStop or something. Sounds like we're a "no" on doing that? I don't have a strong feeling either way.

[Aaronontheweb]

So I updated the DI implementation to call ReleaseActor inside ClearActor, which means all DI resources now get cleaned up upon restart.

Current test results:

cc @akkadotnet/contributors so I need help. We need to determine what the recommended DI configuration is for each DI framework in order to pass these specifications. People can choose not to use that recommendation if they want, but I think this is pretty essential to make clear. These DI frameworks were mostly designed for ASP.NET, and Akka.NET actors can have much longer lifespans than those objects.

[Aaronontheweb]

In order to help, you'll need to clone my branch here https://github.com/Aaronontheweb/akka.net/tree/di-testkit-squash and send PRs to it - wouldn't be the first time we've done something like that :p

[jcwrequests]

@Aaronontheweb On castle you can try setting the release policy not to track along with registration of transient which you already doing. Check this link. http://stackoverflow.com/questions/85183/windsor-container-how-to-force-dispose-of-an-object

[Aaronontheweb]

@jcwrequests senor @stefansedich just left a comment on #942 which explains all of the container configurations he used to get the tests to pass, plus some bug fixes. Mind taking a look through those and let me know if you approve of setting those as the "recommended figuration" for working with Akka.NET actors and DI?

[jcwrequests]

@Aaronontheweb I just saw that. On my phone at the moment but I will see what I can do.Did you try his changes against your tests? He certainly knows his stuff.

[Aaronontheweb]

@jcwrequests yep, got a green check mark now!

[jcwrequests]

Awesome. That Aussie rocks. I will double check but He really knows his way around DI and I can't imagine and issues.

[rogeralsing]

Where is the thread with the more detaled discussion? has that been closed?

[stefansedich]

Should be over in #942 @rogeralsing.

@Aaronontheweb we ready to just YOLO this one and merge it in? I will get onto the doco side of it as soon as I can.

[Aaronontheweb]

@stefansedich I would - any longer term discussions about the role of DI in Akka.NET aren't relevant to the scope of this PR. Ship it!

[Stuey2]

I'd be interested to see how this unofficial DI container compares: https://simpleinjector.org/index.html

Bindings: https://github.com/AkkaNetContrib/Akka.DI.SimpleInjector