WithRemoting should only override declared configurations (#108)

akkadotnet · Sep 16, 2022 · cf7c95d · cf7c95d
1 parent a6af1b0
commit cf7c95d
Show file tree

Hide file tree

Showing 2 changed files with 119 additions and 17 deletions.
diff --git a/src/Akka.Remote.Hosting.Tests/RemoteConfigurationSpecs.cs b/src/Akka.Remote.Hosting.Tests/RemoteConfigurationSpecs.cs
@@ -10,6 +10,93 @@ namespace Akka.Remote.Hosting.Tests;
 
 public class RemoteConfigurationSpecs
 {
+    [Fact(DisplayName = "Empty WithRemoting should return default remoting settings")]
+    public async Task EmptyWithRemotingConfigTest()
+    {
+        // arrange
+        using var host = new HostBuilder().ConfigureServices(services =>
+        {
+            services.AddAkka("RemoteSys", (builder, provider) =>
+            {
+                builder.WithRemoting();
+            });
+        }).Build();
+
+        // act
+        await host.StartAsync();
+        var actorSystem = (ExtendedActorSystem)host.Services.GetRequiredService<ActorSystem>();
+        var config = actorSystem.Settings.Config;
+        var adapters = config.GetStringList("akka.remote.enabled-transports");
+        var tcpConfig = config.GetConfig("akka.remote.dot-netty.tcp");
+
+        // assert
+        adapters.Count.Should().Be(1);
+        adapters[0].Should().Be("akka.remote.dot-netty.tcp");
+
+        tcpConfig.GetString("hostname").Should().BeEmpty();
+        tcpConfig.GetInt("port").Should().Be(2552);
+        tcpConfig.GetString("public-hostname").Should().BeEmpty();
+        tcpConfig.GetInt("public-port").Should().Be(0);
+    }
+
+    [Fact(DisplayName = "WithRemoting should override remote settings")]
+    public async Task WithRemotingConfigTest()
+    {
+        // arrange
+        using var host = new HostBuilder().ConfigureServices(services =>
+        {
+            services.AddAkka("RemoteSys", (builder, provider) =>
+            {
+                builder.WithRemoting("0.0.0.0", 0, "localhost", 12345);
+            });
+        }).Build();
+
+        // act
+        await host.StartAsync();
+        var actorSystem = (ExtendedActorSystem)host.Services.GetRequiredService<ActorSystem>();
+        var config = actorSystem.Settings.Config;
+        var adapters = config.GetStringList("akka.remote.enabled-transports");
+        var tcpConfig = config.GetConfig("akka.remote.dot-netty.tcp");
+
+        // assert
+        adapters.Count.Should().Be(1);
+        adapters[0].Should().Be("akka.remote.dot-netty.tcp");
+
+        tcpConfig.GetString("hostname").Should().Be("0.0.0.0");
+        tcpConfig.GetInt("port").Should().Be(0);
+        tcpConfig.GetString("public-hostname").Should().Be("localhost");
+        tcpConfig.GetInt("public-port").Should().Be(12345);
+    }
+
+    [Fact(DisplayName = "WithRemoting should override remote settings that are overriden")]
+    public async Task WithRemotingConfigOverrideTest()
+    {
+        // arrange
+        using var host = new HostBuilder().ConfigureServices(services =>
+        {
+            services.AddAkka("RemoteSys", (builder, provider) =>
+            {
+                builder.WithRemoting(publicHostname: "localhost", publicPort:12345);
+            });
+        }).Build();
+
+        // act
+        await host.StartAsync();
+        var actorSystem = (ExtendedActorSystem)host.Services.GetRequiredService<ActorSystem>();
+        var config = actorSystem.Settings.Config;
+        var adapters = config.GetStringList("akka.remote.enabled-transports");
+        var tcpConfig = config.GetConfig("akka.remote.dot-netty.tcp");
+
+        // assert
+        adapters.Count.Should().Be(1);
+        adapters[0].Should().Be("akka.remote.dot-netty.tcp");
+
+        tcpConfig.GetString("hostname").Should().BeEmpty();
+        tcpConfig.GetInt("port").Should().Be(2552);
+        tcpConfig.GetString("public-hostname").Should().Be("localhost");
+        tcpConfig.GetInt("public-port").Should().Be(12345);
+    }
+
     [Fact]
     public async Task AkkaRemoteShouldUsePublicHostnameCorrectly()
     {
@@ -24,7 +111,7 @@ public async Task AkkaRemoteShouldUsePublicHostnameCorrectly()
 
         // act
         await host.StartAsync();
-        ExtendedActorSystem actorSystem = (ExtendedActorSystem)host.Services.GetRequiredService<ActorSystem>();
+        var actorSystem = (ExtendedActorSystem)host.Services.GetRequiredService<ActorSystem>();
 
         // assert
         actorSystem.Provider.DefaultAddress.Host.Should().Be("localhost");

diff --git a/src/Akka.Remote.Hosting/AkkaRemoteHostingExtensions.cs b/src/Akka.Remote.Hosting/AkkaRemoteHostingExtensions.cs
@@ -1,27 +1,37 @@
-using Akka.Actor;
+using System.Text;
+using Akka.Actor;
 using Akka.Hosting;
-using Akka.Util;
 
 namespace Akka.Remote.Hosting
 {
     public static class AkkaRemoteHostingExtensions
     {
-        private static AkkaConfigurationBuilder BuildRemoteHocon(this AkkaConfigurationBuilder builder, string hostname, int port, string publicHostname = null, int? publicPort = null)
+        private static AkkaConfigurationBuilder BuildRemoteHocon(
+            this AkkaConfigurationBuilder builder,
+            string hostname = null,
+            int? port = null,
+            string publicHostname = null,
+            int? publicPort = null)
         {
-            if (string.IsNullOrEmpty(publicHostname))
-            {
-                publicHostname = hostname;
-                hostname = "0.0.0.0"; // bind to all addresses by default
-            }
-            var config = $@"
-            akka.remote.dot-netty.tcp.hostname = ""{hostname}""
-            akka.remote.dot-netty.tcp.public-hostname = ""{publicHostname ?? hostname}""
-            akka.remote.dot-netty.tcp.port = {port}
-            akka.remote.dot-netty.tcp.public-port = {publicPort ?? port}
-        ";
+            var sb = new StringBuilder();
+
+            if (!string.IsNullOrWhiteSpace(hostname))
+                sb.AppendFormat("hostname = {0}\n", hostname);
+            if (port != null)
+                sb.AppendFormat("port = {0}\n", port);
+            if(!string.IsNullOrWhiteSpace(publicHostname))
+                sb.AppendFormat("public-hostname = {0}\n", publicHostname);
+            if(publicPort != null)
+                sb.AppendFormat("public-port = {0}\n", publicPort);
+
+            if (sb.Length == 0) 
+                return builder;
+
+            sb.Insert(0, "akka.remote.dot-netty.tcp {\n");
+            sb.Append("}");
 
             // prepend the remoting configuration to the front
-            return builder.AddHocon(config, HoconAddMode.Prepend);
+            return builder.AddHocon(sb.ToString(), HoconAddMode.Prepend);
         }
 
         /// <summary>
@@ -33,7 +43,12 @@ private static AkkaConfigurationBuilder BuildRemoteHocon(this AkkaConfigurationB
         /// <param name="publicHostname">Optional. If using hostname aliasing, this is the host we will advertise.</param>
         /// <param name="publicPort">Optional. If using port aliasing, this is the port we will advertise.</param>
         /// <returns>The same <see cref="AkkaConfigurationBuilder"/> instance originally passed in.</returns>
-        public static AkkaConfigurationBuilder WithRemoting(this AkkaConfigurationBuilder builder, string hostname, int port, string publicHostname = null, int? publicPort = null)
+        public static AkkaConfigurationBuilder WithRemoting(
+            this AkkaConfigurationBuilder builder,
+            string hostname = null,
+            int? port = null,
+            string publicHostname = null,
+            int? publicPort = null)
         {
             var hoconBuilder = BuildRemoteHocon(builder, hostname, port, publicHostname, publicPort);