-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Better integration with Akka.DependencyInjection #169
Conversation
close akkadotnet#144 - working on exposing `IDependencyResovler` everywhere
So we have some neat new APIs now to close out #144 Actors in General// arrange
using var host = await StartHost(collection =>
{
collection.AddAkka("MyActorSys", (builder, sp) =>
{
builder.WithActors((system, registry, resolver) =>
{
var singletonActor = system.ActorOf(resolver.Props<SingletonActor>(), "singleton");
registry.TryRegister<SingletonActor>(singletonActor);
});
});
});
// act
var singletonInstance = host.Services.GetRequiredService<IMySingletonInterface>();
var singletonActor = host.Services.GetRequiredService<ActorRegistry>().Get<SingletonActor>();
var singletonFromActor =
await singletonActor.Ask<IMySingletonInterface>(SingletonActor.GetSingleton.Instance, TimeSpan.FromSeconds(3));
// assert
singletonFromActor.Should().Be(singletonInstance); The third parameter on this new overload for Akka.Cluster.ShardingAll of the public static AkkaConfigurationBuilder WithShardRegion<TKey>(
this AkkaConfigurationBuilder builder,
string typeName,
Func<ActorSystem, IActorRegistry, IDependencyResolver, Func<string, Props>> entityPropsFactory,
IMessageExtractor messageExtractor,
ShardOptions shardOptions)
{
return builder.WithShardRegion<TKey>(typeName, entityPropsFactory,
messageExtractor.ToExtractEntityId(), messageExtractor.ToExtractShardId(), shardOptions);
} We now accept an overload for creating the entity actor Akka.Cluster.Tools.SingletonWe now accept an overload for creating the singleton actor (don't mind the testing infrastructure) protected override void ConfigureServices(HostBuilderContext context, IServiceCollection services)
{
services.AddSingleton<IMyThing>(new ThingImpl("foo1"));
base.ConfigureServices(context, services);
}
protected override void ConfigureAkka(AkkaConfigurationBuilder builder, IServiceProvider provider)
{
builder.ConfigureHost(configurationBuilder =>
{
configurationBuilder.WithSingleton<MySingletonDiActor>("my-singleton",
(_, _, dependencyResolver) => dependencyResolver.Props<MySingletonDiActor>());
}, new ClusterOptions(){ Roles = new[] { "my-host" }}, _tcs, Output!);
}
[Fact]
public async Task Should_launch_ClusterSingletonAndProxy_with_DI_delegate()
{
// arrange
await _tcs.Task; // wait for cluster to start
var registry = Host.Services.GetRequiredService<ActorRegistry>();
var singletonProxy = registry.Get<MySingletonDiActor>();
var thing = Host.Services.GetRequiredService<IMyThing>();
// act
// verify round-trip to the singleton proxy and back
var respond = await singletonProxy.Ask<string>("hit", TimeSpan.FromSeconds(3));
// assert
respond.Should().Be(thing.ThingId);
} I think this should make it a lot easier to use Akka.DependencyInjection going forward. I hope you agree @kpko |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Completed a self-review of changes
@@ -17,5 +17,6 @@ | |||
|
|||
<ItemGroup> | |||
<ProjectReference Include="..\Akka.Cluster.Hosting\Akka.Cluster.Hosting.csproj" /> | |||
<ProjectReference Include="..\Akka.Hosting.TestKit\Akka.Hosting.TestKit.csproj" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decided to use the Akka.Hosting.TestKit in some places.
} | ||
|
||
[Fact] | ||
public async Task Should_launch_ClusterSingletonAndProxy_with_DI_delegate() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validate that we can start singletons with DI and resolve dependencies correctly afterwards
@@ -13,6 +13,32 @@ namespace Akka.Cluster.Hosting.Tests; | |||
|
|||
public static class TestHelper | |||
{ | |||
|
|||
public static void ConfigureHost(this AkkaConfigurationBuilder builder, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored TestHelper
in order to use it with the Akka.Hosting.TestKit
@@ -9,6 +9,7 @@ | |||
|
|||
<ItemGroup> | |||
<PackageReference Include="Akka.TestKit.Xunit2" Version="$(AkkaVersion)" /> | |||
<PackageReference Include="Microsoft.Bcl.AsyncInterfaces" Version="7.0.0" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got an error inside Akka.Cluster.Hosting.Tests claiming that this assembly (Microsoft.Bcl.AsyncInterfaces) could not be found, so I added it to the TestKit project. If this was an err I can try adding it to the tests and not the shared library. I added it to the library because I suspected that other users might run into this problem.
Amazingly enough, I've used the TestKit package in my own apps (which also target .NET 6.0) and never ran into this problem. I assumed that the problem in this solution was that Akka.Hosting.TestKit targets .NET Standard 2.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Microsoft.Bcl.AsyncInterfaces have a severe backward compatibility with netstandard2.0 target, I had to write my own .FirstOrDefault()
in Discovery.Azure
to avoid that problem.
/// <param name="shardOptions">The options to use.</param> | ||
/// <param name="system">The current <see cref="ActorSystem"/>.</param> | ||
/// <returns>A fully populated <see cref="ClusterShardingSettings"/> instance for use with a specific <see cref="ShardRegion"/>.</returns> | ||
private static ClusterShardingSettings PopulateClusterShardingSettings(ShardOptions shardOptions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Standardized creation of ClusterShardingSettings
ClusterSingletonOptions? options = null, | ||
bool createProxyToo = true) | ||
{ | ||
return builder.WithSingleton<TKey>(singletonName, (_, _, _) => actorProps, options, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original cluster singleton API - now calls the new method under the covers.
// act | ||
var singletonInstance = host.Services.GetRequiredService<IMySingletonInterface>(); | ||
var singletonActor = host.Services.GetRequiredService<ActorRegistry>().Get<SingletonActor>(); | ||
var singletonFromActor = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Integration test for basic actor DI support
@@ -112,9 +113,9 @@ public sealed class AkkaConfigurationBuilder | |||
/// </summary> | |||
internal Option<ActorSystem> Sys { get; set; } = Option<ActorSystem>.None; | |||
|
|||
private readonly List<ActorStarter> _actorStarters = new List<ActorStarter>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C# cleanup
@@ -53,6 +52,8 @@ public enum HoconAddMode | |||
/// Delegate used to instantiate <see cref="IActorRef"/>s once the <see cref="ActorSystem"/> has booted. | |||
/// </summary> | |||
public delegate Task ActorStarter(ActorSystem system, IActorRegistry registry); | |||
|
|||
public delegate Task ActorStarterWithResolver(ActorSystem system, IActorRegistry registry, IDependencyResolver resolver); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new delegate type, since I didn't want to break backwards compat with everyone using the original ActorStarter
(even though we're working on maturing the API). I didn't see any harm in adding a new delegate type here.
/// <remarks> | ||
/// This method supports Akka.DependencyInjection directly by making the <see cref="ActorSystem"/>'s <see cref="IDependencyResolver"/> immediately available. | ||
/// </remarks> | ||
public static AkkaConfigurationBuilder WithActors(this AkkaConfigurationBuilder builder, Action<ActorSystem, IActorRegistry, IDependencyResolver> actorStarter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New overloads for exposing the IDependencyResolver
to users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Changes
close #144 - working on improving the accessibility of the
IDependencyResolver
from Akka.DependencyInjection inside all methods responsible for instantiating actors.Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):