Skip to content

Commit

Permalink
Fix Akka.Discovery ServiceDiscovery instantiation (#7245)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Arkatufus
Arkatufus committed Jun 11, 2024
1 parent 3bd219e commit a14bb84
Show file tree
Hide file tree
Showing 2 changed files with 167 additions and 22 deletions.
115 changes: 111 additions & 4 deletions src/core/Akka.Discovery.Tests/DiscoveryConfigurationSpec.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Akka.Util;
using FluentAssertions;
using Xunit;
using static FluentAssertions.FluentActions;

namespace Akka.Discovery.Tests
{
Expand Down Expand Up @@ -113,10 +114,10 @@ class = ""{className}""
}}
}}").WithFallback(ConfigurationFactory.Load()));

Action discoveryInstance = () => _ = Discovery.Get(sys).Default;
discoveryInstance
Invoking(() => _ = Discovery.Get(sys).Default)
.Should().Throw<TargetInvocationException>()
.WithInnerException<DiscoveryException>();
.WithInnerException<DiscoveryException>()
.WithMessage("oh no");
}

[Fact]
Expand All @@ -131,7 +132,79 @@ public void ServiceDiscovery_should_throw_an_argument_exception_for_not_existing
method = ""{className}""
}}").WithFallback(ConfigurationFactory.Load()));

Assert.Throws<ArgumentException>(() => _ = Discovery.Get(sys).Default);
Invoking(() => _ = Discovery.Get(sys).Default).Should()
.Throw<ArgumentException>()
.WithMessage($"Could not load discovery config from path [akka.discovery.{className}]");
}

[Fact(DisplayName = "Discovery should load ServiceDiscovery class with one parameter")]
public void OneParameterTest()
{
using var sys = ActorSystem.Create(
"DiscoveryConfigurationSpec",
ConfigurationFactory.ParseString($@"
akka.discovery {{
method = akka-mock-inside
akka-mock-inside {{
class = ""{typeof(TestDiscoveryWithOneParam).TypeQualifiedName()}""
}}
}}").WithFallback(ConfigurationFactory.Load()));

var discovery = Discovery.Get(sys).Default;
discovery.Should().BeAssignableTo<TestDiscoveryWithOneParam>();
}

[Fact(DisplayName = "Discovery should load ServiceDiscovery class with two parameters")]
public void TwoParametersTest()
{
using var sys = ActorSystem.Create(
"DiscoveryConfigurationSpec",
ConfigurationFactory.ParseString($@"
akka.discovery {{
method = akka-mock-inside
akka-mock-inside {{
class = ""{typeof(TestDiscoveryWithTwoParam).TypeQualifiedName()}""
}}
}}").WithFallback(ConfigurationFactory.Load()));

var discovery = Discovery.Get(sys).Default;
discovery.Should().BeAssignableTo<TestDiscoveryWithTwoParam>();
}

[Fact(DisplayName = "Discovery should throw if ServiceDiscovery class has invalid parameters")]
public void IllegalParametersTest()
{
using var sys = ActorSystem.Create(
"DiscoveryConfigurationSpec",
ConfigurationFactory.ParseString($@"
akka.discovery {{
method = akka-mock-inside
akka-mock-inside {{
class = ""{typeof(TestDiscoveryWithIllegalParam).TypeQualifiedName()}""
}}
}}").WithFallback(ConfigurationFactory.Load()));

Invoking(() => _ = Discovery.Get(sys).Default).Should()
.Throw<ArgumentException>()
.WithMessage("Illegal akka.discovery.akka-mock-inside.class value or incompatible class!*");
}

[Fact(DisplayName = "Discovery should throw if ServiceDiscovery class does not have any public constructor")]
public void IllegalConstructorTest()
{
using var sys = ActorSystem.Create(
"DiscoveryConfigurationSpec",
ConfigurationFactory.ParseString($@"
akka.discovery {{
method = akka-mock-inside
akka-mock-inside {{
class = ""{typeof(TestDiscoveryWithIllegalCtor).TypeQualifiedName()}""
}}
}}").WithFallback(ConfigurationFactory.Load()));

Invoking(() => _ = Discovery.Get(sys).Default).Should()
.Throw<ArgumentException>()
.WithMessage("Illegal akka.discovery.akka-mock-inside.class value or incompatible class!*");
}
}

Expand All @@ -144,6 +217,40 @@ internal class FakeTestDiscovery : ServiceDiscovery
internal class FakeTestDiscovery2 : FakeTestDiscovery
{ }

internal class TestDiscoveryWithOneParam : ServiceDiscovery
{
public TestDiscoveryWithOneParam(ExtendedActorSystem sys){ }

public override Task<Resolved> Lookup(Lookup lookup, TimeSpan resolveTimeout)=>
Task.FromResult((Resolved)null);
}

internal class TestDiscoveryWithTwoParam : ServiceDiscovery
{
public TestDiscoveryWithTwoParam(ExtendedActorSystem sys, Configuration.Config cfg){ }

public override Task<Resolved> Lookup(Lookup lookup, TimeSpan resolveTimeout)=>
Task.FromResult((Resolved)null);
}

internal class TestDiscoveryWithIllegalParam : ServiceDiscovery
{
public TestDiscoveryWithIllegalParam(Configuration.Config cfg){ }

public override Task<Resolved> Lookup(Lookup lookup, TimeSpan resolveTimeout)=>
Task.FromResult((Resolved)null);
}

internal class TestDiscoveryWithIllegalCtor : ServiceDiscovery
{
private TestDiscoveryWithIllegalCtor(){ }
private TestDiscoveryWithIllegalCtor(ExtendedActorSystem sys){ }
private TestDiscoveryWithIllegalCtor(ExtendedActorSystem sys, Configuration.Config cfg){ }

public override Task<Resolved> Lookup(Lookup lookup, TimeSpan resolveTimeout)=>
Task.FromResult((Resolved)null);
}

internal class DiscoveryException : Exception
{
public DiscoveryException()
Expand Down
74 changes: 56 additions & 18 deletions src/core/Akka.Discovery/Discovery.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
using Akka.Annotations;
using Akka.Configuration;
using Akka.Event;
using Akka.Util;
using BindingFlags = System.Reflection.BindingFlags;

namespace Akka.Discovery
{
Expand Down Expand Up @@ -71,27 +71,65 @@ private ServiceDiscovery CreateServiceDiscovery(string method)
if(!config.HasPath("class"))
throw new ArgumentException($"akka.discovery.{method} must contain field `class` that is a FQN of an `Akka.Discovery.ServiceDiscovery` implementation");

Try<ServiceDiscovery> Create(string typeName)
{
var dynamic = DynamicAccess.CreateInstanceFor<ServiceDiscovery>(typeName, _system);
return dynamic.RecoverWith(ex => ex is TypeLoadException or MissingMethodException
? DynamicAccess.CreateInstanceFor<ServiceDiscovery>(typeName)
: dynamic);
}

var className = config.GetString("class");
_log.Info($"Starting Discovery service using [{method}] method, class: [{className}]");
var instanceTry = Create(className);

return instanceTry.IsSuccess switch
try
{
return Create(className);
}
catch (Exception ex)
{
if (ex is TypeLoadException or MissingMethodException)
throw new ArgumentException(
message: $"Illegal akka.discovery.{method}.class value or incompatible class!\n" +
"The implementation class MUST extend Akka.Discovery.ServiceDiscovery with:\n" +
" * parameterless constructor, " +
$" * constructor with a single {nameof(ExtendedActorSystem)} parameter, or\n" +
$" * constructor with {nameof(ExtendedActorSystem)} and {nameof(Configuration.Config)} parameters.",
paramName: nameof(method),
innerException: ex);
throw;
}

ServiceDiscovery Create(string typeName)
{
true => instanceTry.Get(),
false when instanceTry.Failure.Value is TypeLoadException or MissingMethodException =>
throw new ArgumentException(nameof(method), $"Illegal akka.discovery.{method}.class value or incompatible class! \n" +
"The implementation class MUST extend Akka.Discovery.ServiceDiscovery and take an \n" +
"ExtendedActorSystem as constructor argument."),
_ => throw instanceTry.Failure.Value
};
var type = Type.GetType(typeName: typeName);
if (type is null || !typeof(ServiceDiscovery).IsAssignableFrom(type))
throw new TypeLoadException();

var bindFlags = BindingFlags.Instance | BindingFlags.Public;

var ctor = type.GetConstructor(
bindingAttr: bindFlags,
binder: null,
types: new[]
{
typeof(ExtendedActorSystem),
typeof(Configuration.Config)
},
modifiers: null);
if (ctor is not null)
return (ServiceDiscovery) Activator.CreateInstance(type, _system, config);

ctor = type.GetConstructor(
bindingAttr: bindFlags,
binder: null,
types: new[] { typeof(ExtendedActorSystem) },
modifiers: null);
if (ctor is not null)
return (ServiceDiscovery) Activator.CreateInstance(type, _system);

ctor = type.GetConstructor(
bindingAttr: bindFlags,
binder: null,
types: Array.Empty<Type>(),
modifiers: null);
if (ctor is null)
throw new MissingMethodException();

return (ServiceDiscovery) Activator.CreateInstance(type);
}
}

public static Discovery Get(ActorSystem system) => system.WithExtension<Discovery, DiscoveryProvider>();
Expand Down

0 comments on commit a14bb84

Please sign in to comment.