Skip to content
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

ServiceProviderProps and Props Changes #4858

Merged
merged 13 commits into from
Mar 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Linq;
using Akka.Actor;
using Akka.Configuration;
using Akka.Routing;
using Akka.TestKit;
using FluentAssertions;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -162,6 +163,37 @@ public void ActorsWithNonDiDependenciesShouldStart()
deps1Single.Should().Be(deps2Single);
}

[Fact(DisplayName = "Props created via the ServiceProvider should support the standard Props copying methods")]
public void ServiceProvider_Props_should_support_copying()
{
// <CreateNonDiActor>
var spExtension = ServiceProvider.For(Sys);
var arg1 = "foo";
var arg2 = "bar";
var props = spExtension.Props<NonDiArgsActor>(arg1, arg2).WithRouter(new RoundRobinPool(10).WithSupervisorStrategy(new OneForOneStrategy(
ex =>
{
TestActor.Tell(ex);
return Directive.Restart;
})));

// create a scoped round robin pool using the props from Akka.DependencyInjection
var scoped1 = Sys.ActorOf(props, "scoped1");

// validate that non-DI'd arguments were passed to actor constructor arguments correctly
scoped1.Tell("fetch");
ExpectMsg<string>().Should().Be(arg1);
ExpectMsg<string>().Should().Be(arg2);

// validate that this is a router
scoped1.Tell(GetRoutees.Instance);
ExpectMsg<Routees>().Members.Count().Should().Be(10);

// validate that the router's supervision strategy has been enforced
scoped1.Tell(new Crash());
ExpectMsg<ApplicationException>();
}

public class Crash { }

public class FetchDependencies { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,6 @@ public static ServiceProvider For(ActorSystem actorSystem)
return actorSystem.WithExtension<ServiceProvider, ServiceProviderExtension>();
}

///// <summary>
///// Uses a delegate to dynamically instantiate an actor where some of the constructor arguments are populated via dependency injection
///// and others are not.
///// </summary>
///// <remarks>
///// YOU ARE RESPONSIBLE FOR MANAGING THE LIFECYCLE OF YOUR OWN DEPENDENCIES. AKKA.NET WILL NOT ATTEMPT TO DO IT FOR YOU.
///// </remarks>
///// <typeparam name="T">The type of actor to instantiate.</typeparam>
///// <param name="producer">The delegate used to create a new instance of your actor type.</param>
///// <returns>A new <see cref="Props"/> instance which uses DI internally.</returns>
//public Props Props<T>(Func<IServiceProvider, T> producer) where T : ActorBase
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented out method - just removed it.

//{
// return new ServiceProviderProps<T>(producer, Provider);
//}

/// <summary>
/// Uses a delegate to dynamically instantiate an actor where some of the constructor arguments are populated via dependency injection
/// and others are not.
Expand All @@ -74,7 +59,7 @@ public static ServiceProvider For(ActorSystem actorSystem)
/// <returns>A new <see cref="Akka.Actor.Props"/> instance which uses DI internally.</returns>
public Props Props<T>(params object[] args) where T : ActorBase
{
return new ServiceProviderProps<T>(Provider, args);
return Akka.Actor.Props.CreateBy(new ServiceProviderActorProducer<T>(Provider, args));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the new Props.CreateBy method with takes an IIndirectActorProducer reference directly.

}
}

Expand All @@ -99,51 +84,33 @@ public override ServiceProvider CreateExtension(ExtendedActorSystem system)
}

/// <summary>
/// This class represents a specialized <see cref="Akka.Actor.Props"/> that uses delegate invocation
/// to create new actor instances, rather than a traditional <see cref="System.Activator"/>.
/// INTERNAL API
///
/// Relies on having an active <see cref="IServiceProvider"/> implementation available
/// Used to create actors via the <see cref="ActivatorUtilities"/>.
/// </summary>
/// <typeparam name="TActor">The type of the actor to create.</typeparam>
internal class ServiceProviderProps<TActor> : Props where TActor : ActorBase
/// <typeparam name="TActor">the actor type</typeparam>
internal sealed class ServiceProviderActorProducer<TActor> : IIndirectActorProducer where TActor:ActorBase
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor the ServiceProviderProps into an IIndirectActorProducer that can be used directly inside the Props class.

{
private readonly IServiceProvider _provider;
private readonly object[] _args;

/// <summary>
/// Initializes a new instance of the <see cref="ServiceProviderProps{TActor}" /> class.
/// </summary>
/// <param name="provider">The <see cref="IServiceProvider"/> used to power this class</param>
/// <param name="args">The constructor arguments passed to the actor's constructor.</param>
public ServiceProviderProps(IServiceProvider provider, params object[] args)
: base(typeof(TActor), args)
public ServiceProviderActorProducer(IServiceProvider provider, object[] args)
{
_provider = provider;
_args = args;
ActorType = typeof(TActor);
}

/// <summary>
/// Creates a new actor using the configured factory method.
/// </summary>
/// <returns>The actor created using the factory method.</returns>
public override ActorBase NewActor()
public ActorBase Produce()
{
return ActivatorUtilities.CreateInstance<TActor>(_provider, Arguments);
return (ActorBase)ActivatorUtilities.CreateInstance(_provider, ActorType, _args);
}

#region Copy methods
public Type ActorType { get; }

/// <summary>
/// Creates a copy of the current instance.
/// </summary>
/// <returns>The newly created <see cref="Akka.Actor.Props"/></returns>
protected override Props Copy()
public void Release(ActorBase actor)
{
return new ServiceProviderProps<TActor>(_provider, Arguments)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no-op for this class - scopes are meant to be managed directly by the users.

{
Deploy = Deploy,
SupervisorStrategy = SupervisorStrategy
};
// no-op
}

#endregion
}
}
3 changes: 3 additions & 0 deletions src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1408,8 +1408,11 @@ namespace Akka.Actor
public static Akka.Actor.Props Create<TActor>(Akka.Actor.SupervisorStrategy supervisorStrategy)
where TActor : Akka.Actor.ActorBase, new () { }
public static Akka.Actor.Props Create(System.Type type, params object[] args) { }
[System.ObsoleteAttribute("Do not use this method. Call CreateBy(IIndirectActorProducer, params object[] arg" +
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All public API changes made to the Props class - kept it to a bare minimum for compatibility reasons.

"s) instead")]
public static Akka.Actor.Props CreateBy<TProducer>(params object[] args)
where TProducer : class, Akka.Actor.IIndirectActorProducer { }
public static Akka.Actor.Props CreateBy(Akka.Actor.IIndirectActorProducer producer, params object[] args) { }
public bool Equals(Akka.Actor.Props other) { }
public override bool Equals(object obj) { }
public override int GetHashCode() { }
Expand Down
25 changes: 18 additions & 7 deletions src/core/Akka.Remote.Tests/RemotingSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -411,8 +411,7 @@ public void Remoting_must_create_by_IndirectActorProducer()
{
try
{
Resolve.SetResolver(new TestResolver());
var r = Sys.ActorOf(Props.CreateBy<Resolve<Echo2>>(), "echo");
var r = Sys.ActorOf(Props.CreateBy(new TestResolver<Echo2>()), "echo");
Assert.Equal("akka.test://remote-sys@localhost:12346/remote/akka.test/RemotingSpec@localhost:12345/user/echo", r.Path.ToString());
}
finally
Expand All @@ -426,8 +425,7 @@ public void Remoting_must_create_by_IndirectActorProducer_and_ping()
{
try
{
Resolve.SetResolver(new TestResolver());
var r = Sys.ActorOf(Props.CreateBy<Resolve<Echo2>>(), "echo");
var r = Sys.ActorOf(Props.CreateBy(new TestResolver<Echo2>()), "echo");
Assert.Equal("akka.test://remote-sys@localhost:12346/remote/akka.test/RemotingSpec@localhost:12345/user/echo", r.Path.ToString());
r.Tell("ping", TestActor);
ExpectMsg(("pong", TestActor), TimeSpan.FromSeconds(1.5));
Expand Down Expand Up @@ -902,11 +900,24 @@ protected override void OnReceive(object message)
}
}

class TestResolver : IResolver
class TestResolver<TActor> : IIndirectActorProducer where TActor:ActorBase
{
public T Resolve<T>(object[] args)
public Type ActorType => typeof(TActor);
private readonly object[] _args;

public TestResolver(params object[] args)
{
_args = args;
}

public ActorBase Produce()
{
return (ActorBase)Activator.CreateInstance(ActorType, _args);
}

public void Release(ActorBase actor)
{
return Activator.CreateInstance(typeof(T), args).AsInstanceOf<T>();

}
}

Expand Down
2 changes: 1 addition & 1 deletion src/core/Akka.Tests/Actor/PropsSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public void Props_must_create_actor_by_producer()
{
TestLatch latchProducer = new TestLatch();
TestLatch latchActor = new TestLatch();
var props = Props.CreateBy<TestProducer>(latchProducer, latchActor);
var props = Props.CreateBy(new TestProducer(latchProducer, latchActor));
IActorRef actor = Sys.ActorOf(props);
latchActor.Ready(TimeSpan.FromSeconds(1));
}
Expand Down
Loading