feat(mesh): MeshCommand envelope + generation-suffixed ClusterClient factory

Phase 2 Task 2. Two small pieces the comm actors are built on.

MeshCommand carries a command plus the DPS topic it would have been published
on, so the admin singletons stop knowing which transport is in force. Without
it the dark switch would have to be threaded through five publish sites across
two actors, each free to drift. Central-side only -- it never crosses the wire,
so it carries no serialization contract.

DefaultMeshClusterClientFactory appends a generation counter to the actor name.
Context.Stop is asynchronous and the name stays reserved until termination
completes, so recreating the same name inside one message handler throws
InvalidActorNameException -- which is exactly what a contact-set change does
(stop the old client, create the replacement, one handler). Ported from the
sister project, where it was a shipped bug fix rather than foresight.

Sabotage-verified: freezing the name to a constant reddens both name tests with
InvalidActorNameException.

Dropped a third test I had written -- it asserted ClusterClientSettings
directly and never touched the factory, so it was coverage theatre. Replaced
with a comment saying where contact propagation IS covered (the Task 8 boundary
test, the only place it can actually fail).

Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
Joseph Doherty
2026-07-22 10:45:13 -04:00
parent 7b71a6a35d
commit 0d6669d5ff
3 changed files with 142 additions and 0 deletions
@@ -0,0 +1,28 @@
namespace ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh;
/// <summary>
/// A central→node command, plus the DistributedPubSub topic it would have been published on.
/// </summary>
/// <param name="Topic">
/// The legacy DPS topic. Carried so the router can publish under
/// <c>MeshTransport:Mode = Dps</c> without a second dispatch table; ignored entirely under
/// <c>ClusterClient</c> mode, where the destination is an actor path rather than a topic.
/// </param>
/// <param name="Message">
/// The command itself, never wrapped. Node-side handlers see exactly the type they see today,
/// which is what lets the dark switch touch publishers only.
/// </param>
/// <remarks>
/// <para>
/// This envelope exists so the admin singletons stop knowing which transport is in force.
/// Before per-cluster mesh Phase 2 each publisher held its own
/// <c>DistributedPubSub.Get(Context.System).Mediator</c> call, so the switch would have had
/// to be threaded through five call sites across two actors, and each would have been free
/// to drift.
/// </para>
/// <para>
/// Central-side only — it never crosses the wire, so it carries no serialization contract
/// and needs no additive-evolution discipline.
/// </para>
/// </remarks>
public sealed record MeshCommand(string Topic, object Message);
@@ -0,0 +1,47 @@
using System.Collections.Immutable;
using Akka.Actor;
using Akka.Cluster.Tools.Client;
namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Communication;
/// <summary>
/// Creates the <c>ClusterClient</c> central uses to reach driver nodes. Exists as a seam so
/// <c>CentralCommunicationActor</c> can be tested against a <c>TestProbe</c> without standing up
/// a second cluster.
/// </summary>
public interface IMeshClusterClientFactory
{
/// <summary>Creates a client for the given receptionist contact points.</summary>
/// <param name="system">The actor system to spawn the client in.</param>
/// <param name="contacts">Receptionist actor paths, one per reachable node.</param>
/// <returns>The new client's <see cref="IActorRef"/>.</returns>
IActorRef Create(ActorSystem system, ImmutableHashSet<ActorPath> contacts);
}
/// <inheritdoc />
public sealed class DefaultMeshClusterClientFactory : IMeshClusterClientFactory
{
/// <summary>
/// Per-incarnation generation counter, appended to the actor name.
/// </summary>
/// <remarks>
/// <c>Context.Stop</c> of the previous client is <b>asynchronous</b> — its actor name stays
/// reserved until termination completes — so recreating the same name while handling a single
/// message throws <see cref="InvalidActorNameException"/>. That is exactly what a contact-set
/// change does: stop the old client, create the replacement, both in one handler. A
/// generation suffix makes every incarnation's name unique by construction. Ported from the
/// sister project, where it was a shipped bug fix rather than foresight.
/// </remarks>
private long _generation;
/// <inheritdoc />
public IActorRef Create(ActorSystem system, ImmutableHashSet<ActorPath> contacts)
{
ArgumentNullException.ThrowIfNull(system);
ArgumentNullException.ThrowIfNull(contacts);
var settings = ClusterClientSettings.Create(system).WithInitialContacts(contacts);
var name = $"mesh-client-{Interlocked.Increment(ref _generation)}";
return system.ActorOf(ClusterClient.Props(settings), name);
}
}
@@ -0,0 +1,67 @@
using System.Collections.Immutable;
using Akka.Actor;
using Akka.Cluster.Tools.Client;
using Akka.Configuration;
using Akka.TestKit.Xunit2;
using Shouldly;
using Xunit;
using ZB.MOM.WW.OtOpcUa.ControlPlane.Communication;
namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Tests.Communication;
/// <summary>
/// Guards the one non-obvious part of client creation: recreating a client with the same name
/// inside a single message handler throws, because <c>Context.Stop</c> is asynchronous and the
/// name stays reserved until termination completes.
/// </summary>
public class MeshClusterClientFactoryTests : 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 = 127.0.0.1")
.WithFallback(ClusterClientReceptionist.DefaultConfig());
public MeshClusterClientFactoryTests()
: base(TestConfig)
{
}
// A real ClusterClient pointed at an unreachable contact is safe in TestKit — it simply retries
// the initial-contact rotation and never fails the test.
private static ImmutableHashSet<ActorPath> UnreachableContact => ImmutableHashSet.Create(
ActorPath.Parse("akka.tcp://otopcua@127.0.0.1:65501/system/receptionist"));
[Fact]
public void Recreating_a_client_in_one_pass_does_not_collide_on_the_actor_name()
{
var factory = new DefaultMeshClusterClientFactory();
var first = factory.Create(Sys, UnreachableContact);
Sys.Stop(first);
// Stop is asynchronous: `first`'s name is still reserved right here. Without the generation
// suffix this throws InvalidActorNameException — which is precisely what a contact-set
// change would do in production, where stop-then-recreate happens in one handler.
var second = factory.Create(Sys, UnreachableContact);
second.Path.Name.ShouldNotBe(first.Path.Name);
}
[Fact]
public void Every_incarnation_gets_a_distinct_name()
{
var factory = new DefaultMeshClusterClientFactory();
var names = Enumerable.Range(0, 5)
.Select(_ => factory.Create(Sys, UnreachableContact).Path.Name)
.ToList();
names.Distinct().Count().ShouldBe(5);
}
// Contact-set propagation is deliberately NOT asserted here: reading initial contacts back
// off a created ClusterClient is not possible, and asserting ClusterClientSettings directly
// would test Akka rather than this factory. It is covered where it can actually fail — the
// real two-ActorSystem boundary test, which only delivers if the contacts reached the client.
}