fix(communication): guard per-site ClusterClient creation; add real-factory address-edit lifecycle test

2026-07-08 14:57:13 -04:00
parent 75cbac4478
commit f91e75bfe4
2 changed files with 97 additions and 1 deletions
@@ -0,0 +1,82 @@
+using System.Collections.Immutable;
+using Akka.Actor;
+using Akka.Cluster.Tools.Client;
+using Akka.Configuration;
+using Akka.TestKit.Xunit2;
+using Microsoft.Extensions.DependencyInjection;
+using NSubstitute;
+using Xunit;
+using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
+using ZB.MOM.WW.ScadaBridge.Communication;
+using ZB.MOM.WW.ScadaBridge.Communication.Actors;
+
+namespace ZB.MOM.WW.ScadaBridge.Communication.Tests;
+
+public class CentralCommunicationActorClientLifecycleTests : TestKit
+{
+    private static readonly Config TestConfig = ConfigurationFactory.ParseString(@"
+        akka.actor.provider = cluster
+        akka.remote.dot-netty.tcp.port = 0
+        akka.remote.dot-netty.tcp.hostname = localhost")
+        .WithFallback(ClusterClientReceptionist.DefaultConfig());
+
+    public CentralCommunicationActorClientLifecycleTests() : base(TestConfig) { }
+
+    // Empty ServiceProvider with a no-op ISiteRepository so the actor's periodic
+    // db-refresh (fired at PreStart) resolves and returns no sites, keeping the
+    // logs clean; the tests drive SiteAddressCacheLoaded directly.
+    private static IServiceProvider EmptyProvider()
+    {
+        var siteRepo = Substitute.For<ISiteRepository>();
+        siteRepo.GetAllSitesAsync(Arg.Any<CancellationToken>()).Returns(new List<Site>());
+        var services = new ServiceCollection();
+        services.AddScoped(_ => siteRepo);
+        return services.BuildServiceProvider();
+    }
+
+    private static SiteAddressCacheLoaded Load(string siteId, params string[] addrs) =>
+        new(new Dictionary<string, IReadOnlyList<string>>
+            { [siteId] = addrs.ToList().AsReadOnly() });
+
+    [Fact]
+    public void AddressEdit_RecreatesClient_WithoutRestartLoop()
+    {
+        var actor = Sys.ActorOf(Props.Create(() => new CentralCommunicationActor(
+            EmptyProvider(), new DefaultSiteClientFactory(), null)));
+
+        // First load creates the client; second load (edited NodeA address) stops
+        // the old one and creates a replacement in the same message handling.
+        // Pre-fix this throws InvalidActorNameException and restarts the actor.
+        EventFilter.Exception<InvalidActorNameException>().Expect(0, () =>
+        {
+            actor.Tell(Load("site-a", "akka.tcp://scadabridge@node-a:8081"));
+            actor.Tell(Load("site-a", "akka.tcp://scadabridge@node-a-edited:8081"));
+            actor.Tell(Load("site-a", "akka.tcp://scadabridge@node-a:8081")); // and back — third generation
+        });
+
+        // The actor must still be alive and routing (not crash-looping):
+        // an envelope for an unknown site produces the "No ClusterClient" warning,
+        // proving the Receive pipeline is healthy.
+        EventFilter.Warning(contains: "No ClusterClient for site").ExpectOne(() =>
+            actor.Tell(new SiteEnvelope("unknown-site", new object())));
+    }
+
+    [Fact]
+    public void FactoryThrow_SkipsSite_DoesNotCrashActor()
+    {
+        var throwingFactory = Substitute.For<ISiteClientFactory>();
+        throwingFactory.Create(Arg.Any<ActorSystem>(), Arg.Any<string>(), Arg.Any<ImmutableHashSet<ActorPath>>())
+            .Returns(_ => throw new InvalidOperationException("boom"));
+        var actor = Sys.ActorOf(Props.Create(() => new CentralCommunicationActor(
+            EmptyProvider(), throwingFactory, null)));
+
+        EventFilter.Error(contains: "Failed to create ClusterClient").ExpectOne(() =>
+            actor.Tell(Load("site-a", "akka.tcp://scadabridge@node-a:8081")));
+
+        // Actor survived — a subsequent envelope for that (unrouted) site still
+        // produces the healthy "No ClusterClient" warning rather than a dead actor.
+        EventFilter.Warning(contains: "No ClusterClient for site").ExpectOne(() =>
+            actor.Tell(new SiteEnvelope("site-a", new object())));
+    }
+}